From dd95131630b3fea9b738e67b9106b2a05470caed Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 8 Oct 2021 14:28:42 +0100 Subject: [PATCH 1/2] C++: Test spacing. --- .../semmle/TOCTOUFilesystemRace.expected | 12 ++--- .../Security/CWE/CWE-367/semmle/test2.cpp | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 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 f0f7aaaefee..6de0b3a68ce 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 @@ -5,9 +5,9 @@ | test2.cpp:130:7:130:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:130:13:130:16 | path | filename | test2.cpp:128:21:128:27 | buf_ptr | checked | | test2.cpp:157:7:157:10 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:157:12:157:15 | path | filename | test2.cpp:155:6:155:9 | call to stat | checked | | test2.cpp:170:7:170:10 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:170:12:170:15 | path | filename | test2.cpp:168:6:168:10 | call to lstat | checked | -| test2.cpp:245:3:245:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:245:9:245:12 | path | filename | test2.cpp:238:6:238:10 | call to fopen | checked | -| test2.cpp:277:7:277:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:277:13:277:16 | path | filename | test2.cpp:275:6:275:11 | call to access | checked | -| test2.cpp:303:7:303:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:303:13:303:16 | path | filename | test2.cpp:301:7:301:12 | call to access | checked | -| test2.cpp:317:7:317:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:317:13:317:16 | path | filename | test2.cpp:313:6:313:11 | call to access | checked | -| test2.cpp:348:3:348:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:348:9:348:12 | path | filename | test2.cpp:341:6:341:10 | call to fopen | checked | -| test2.cpp:356:3:356:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:356:9:356:13 | path2 | filename | test2.cpp:354:7:354:12 | call to rename | checked | +| test2.cpp:297:3:297:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:297:9:297:12 | path | filename | test2.cpp:290:6:290:10 | call to fopen | checked | +| test2.cpp:329:7:329:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:329:13:329:16 | path | filename | test2.cpp:327:6:327:11 | call to access | checked | +| test2.cpp:355:7:355:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:355:13:355:16 | path | filename | test2.cpp:353:7:353:12 | call to access | checked | +| test2.cpp:369:7:369:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:369:13:369:16 | path | filename | test2.cpp:365:6:365:11 | call to access | checked | +| test2.cpp:400:3:400:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:400:9:400:12 | path | filename | test2.cpp:393:6:393:10 | call to fopen | checked | +| test2.cpp:408:3:408:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:408:9:408:13 | path2 | filename | test2.cpp:406:7:406:12 | call to rename | checked | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp index e1317e9a9ca..40b3929f81b 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp @@ -199,6 +199,58 @@ void test2_10(int dir, const char *path, int arg) // ... } + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + // --- open -> stat --- void test3_1(const char *path, int arg) From 1c56573194c00c3b1f44428eb7e628f073dfbc83 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 8 Oct 2021 14:30:27 +0100 Subject: [PATCH 2/2] C++: Add tests. --- .../semmle/TOCTOUFilesystemRace.expected | 4 + .../Security/CWE/CWE-367/semmle/test2.cpp | 84 +++++++++---------- 2 files changed, 46 insertions(+), 42 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 6de0b3a68ce..413d3dbccf9 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 @@ -5,6 +5,10 @@ | test2.cpp:130:7:130:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:130:13:130:16 | path | filename | test2.cpp:128:21:128:27 | buf_ptr | checked | | test2.cpp:157:7:157:10 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:157:12:157:15 | path | filename | test2.cpp:155:6:155:9 | call to stat | checked | | test2.cpp:170:7:170:10 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:170:12:170:15 | path | filename | test2.cpp:168:6:168:10 | call to lstat | checked | +| test2.cpp:209:7:209:10 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:209:12:209:15 | path | filename | test2.cpp:207:6:207:9 | call to stat | checked | +| test2.cpp:228:8:228:11 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:228:13:228:16 | path | filename | test2.cpp:224:6:224:9 | call to stat | checked | +| test2.cpp:228:8:228:11 | call to open | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:228:13:228:16 | path | filename | test2.cpp:226:7:226:9 | buf | checked | +| test2.cpp:249:6:249:10 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:249:12:249:15 | path | filename | test2.cpp:244:6:244:9 | call to stat | checked | | test2.cpp:297:3:297:7 | call to chmod | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:297:9:297:12 | path | filename | test2.cpp:290:6:290:10 | call to fopen | checked | | test2.cpp:329:7:329:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:329:13:329:16 | path | filename | test2.cpp:327:6:327:11 | call to access | checked | | test2.cpp:355:7:355:11 | call to fopen | The $@ being operated upon was previously $@, but the underlying file may have been changed since then. | test2.cpp:355:13:355:16 | path | filename | test2.cpp:353:7:353:12 | call to access | checked | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp index 40b3929f81b..14875b9d367 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-367/semmle/test2.cpp @@ -199,57 +199,57 @@ void test2_10(int dir, const char *path, int arg) // ... } +void test2_11(const char *path, int arg) +{ + stat_data buf; + int f; + if (stat(path, &buf)) + { + f = open(path, arg); // GOOD (here stat is just a redundant check that the file exists / path is valid, confirmed by the return value of open) [FALSE POSITIVE] + if (f == -1) + { + // handle error + } + // ... + } +} +void test2_12(const char *path, int arg) +{ + stat_data buf; + int f; + if (stat(path, &buf)) + { + if (buf.foo == 11) // check a property of the file + { + f = open(path, arg); // BAD + if (f == -1) + { + // handle error + } + } + // ... + } +} +void test2_13(const char *path, int arg) +{ + stat_data buf; + FILE *f; + if (stat(path, &buf)) // check the file does *not* exist + { + return; + } + f = fopen(path, "wt"); // BAD - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // ... +} // --- open -> stat ---