From 6499197087c05ac5cc2ea61766d89a006a89670c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 7 May 2020 17:53:09 +0100 Subject: [PATCH 1/2] C++: Add a test of TOCTOUFilesystemRace.ql. --- .../semmle/TOCTOUFilesystemRace.expected | 3 ++ .../CWE-367/semmle/TOCTOUFilesystemRace.qlref | 1 + .../Security/CWE/CWE-367/semmle/test.cpp | 51 +++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.qlref create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected new file mode 100644 index 000000000000..f514742ff0ac --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected @@ -0,0 +1,3 @@ +| test.cpp:21:3:21:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:21:10:21:14 | file1 | filename | test.cpp:19:7:19:12 | call to rename | checked | +| test.cpp:35:3:35:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:35:10:35:14 | file1 | filename | test.cpp:32:7:32:12 | call to rename | checked | +| test.cpp:49:3:49:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:49:10:49:14 | file1 | filename | test.cpp:47:7:47:12 | call to rename | checked | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.qlref b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.qlref new file mode 100644 index 000000000000..c7d2e9c45f4b --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-367/TOCTOUFilesystemRace.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp new file mode 100644 index 000000000000..b876146f5716 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp @@ -0,0 +1,51 @@ + +class String +{ +public: + String(const char *_s); + void set(const char *_s); +}; + +void create(const String &filename); +bool rename(const String &from, const String &to); +void remove(const String &filename); + +void test1() +{ + String file1 = "a.txt"; + String file2 = "b.txt"; + + create(file1); + if (!rename(file1, file2)) + { + remove(file1); // BAD + } +} + + +void test2() +{ + String file1 = "a.txt"; + String file2 = "b.txt"; + + create(file1); + if (!rename(file1, file2)) + { + file1.set("d.txt"); + remove(file1); // GOOD [FALSE POSITIVE] + } +} + + +void test3() +{ + String file1 = "a.txt"; + String file2 = "b.txt"; + file1.set("d.txt"); + + create(file1); + if (!rename(file1, file2)) + { + remove(file1); // BAD + } +} From bff97d9fe517c9dbba4fc77e0b480b4daf0d522f Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 7 May 2020 18:36:29 +0100 Subject: [PATCH 2/2] C++: Effect of #3382. --- .../Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected | 2 -- cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected index f514742ff0ac..4794d4744af8 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/TOCTOUFilesystemRace.expected @@ -1,3 +1 @@ | test.cpp:21:3:21:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:21:10:21:14 | file1 | filename | test.cpp:19:7:19:12 | call to rename | checked | -| test.cpp:35:3:35:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:35:10:35:14 | file1 | filename | test.cpp:32:7:32:12 | call to rename | checked | -| test.cpp:49:3:49:8 | call to remove | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test.cpp:49:10:49:14 | file1 | filename | test.cpp:47:7:47:12 | call to rename | checked | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp index b876146f5716..6433523d69a0 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test.cpp @@ -32,7 +32,7 @@ void test2() if (!rename(file1, file2)) { file1.set("d.txt"); - remove(file1); // GOOD [FALSE POSITIVE] + remove(file1); // GOOD } } @@ -46,6 +46,6 @@ void test3() create(file1); if (!rename(file1, file2)) { - remove(file1); // BAD + remove(file1); // BAD [NOT DETECTED] } }