diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 4e626d0bc773..94a9cacf9f49 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -88,6 +88,11 @@ module TaintedPathConfig implements DataFlow::ConfigSig { hasUpperBoundsCheck(checkedVar) ) } + + predicate isBarrierOut(DataFlow::Node node) { + // make sinks barriers so that we only report the closest instance + isSink(node) + } } module TaintedPath = TaintTracking::Global; diff --git a/cpp/ql/src/change-notes/2024-04-25-path-injection.md b/cpp/ql/src/change-notes/2024-04-25-path-injection.md new file mode 100644 index 000000000000..9989a7dc6221 --- /dev/null +++ b/cpp/ql/src/change-notes/2024-04-25-path-injection.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Uncontrolled data used in path expression" query (`cpp/path-injection`) query produces fewer near-duplicate results. diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected index 1a1bf6081e21..f8b96c81b1eb 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected @@ -1,11 +1,16 @@ edges | test.c:8:27:8:30 | **argv | test.c:9:23:9:29 | *access to array | provenance | | | test.c:8:27:8:30 | **argv | test.c:31:22:31:28 | *access to array | provenance | | -| test.c:8:27:8:30 | **argv | test.c:57:10:57:16 | *access to array | provenance | | +| test.c:8:27:8:30 | **argv | test.c:69:14:69:20 | *access to array | provenance | | | test.c:9:23:9:29 | *access to array | test.c:17:11:17:18 | *fileName | provenance | TaintFunction | | test.c:31:22:31:28 | *access to array | test.c:32:11:32:18 | *fileName | provenance | | | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | *fileName | provenance | | | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | *fileName | provenance | | +| test.c:48:21:48:26 | *call to getenv | test.c:48:21:48:26 | *call to getenv | provenance | | +| test.c:48:21:48:26 | *call to getenv | test.c:49:11:49:17 | *tainted | provenance | | +| test.c:54:21:54:26 | *call to getenv | test.c:55:11:55:16 | *buffer | provenance | TaintFunction | +| test.c:74:13:74:18 | read output argument | test.c:76:11:76:16 | *buffer | provenance | | +| test.c:75:13:75:18 | read output argument | test.c:76:11:76:16 | *buffer | provenance | | nodes | test.c:8:27:8:30 | **argv | semmle.label | **argv | | test.c:9:23:9:29 | *access to array | semmle.label | *access to array | @@ -16,11 +21,23 @@ nodes | test.c:38:11:38:18 | *fileName | semmle.label | *fileName | | test.c:43:17:43:24 | scanf output argument | semmle.label | scanf output argument | | test.c:44:11:44:18 | *fileName | semmle.label | *fileName | -| test.c:57:10:57:16 | *access to array | semmle.label | *access to array | +| test.c:48:21:48:26 | *call to getenv | semmle.label | *call to getenv | +| test.c:48:21:48:26 | *call to getenv | semmle.label | *call to getenv | +| test.c:49:11:49:17 | *tainted | semmle.label | *tainted | +| test.c:54:21:54:26 | *call to getenv | semmle.label | *call to getenv | +| test.c:55:11:55:16 | *buffer | semmle.label | *buffer | +| test.c:69:14:69:20 | *access to array | semmle.label | *access to array | +| test.c:74:13:74:18 | read output argument | semmle.label | read output argument | +| test.c:75:13:75:18 | read output argument | semmle.label | read output argument | +| test.c:76:11:76:16 | *buffer | semmle.label | *buffer | subpaths #select | test.c:17:11:17:18 | fileName | test.c:8:27:8:30 | **argv | test.c:17:11:17:18 | *fileName | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:8:27:8:30 | **argv | user input (a command-line argument) | | test.c:32:11:32:18 | fileName | test.c:8:27:8:30 | **argv | test.c:32:11:32:18 | *fileName | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:8:27:8:30 | **argv | user input (a command-line argument) | | test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | *fileName | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | scanf output argument | user input (value read by scanf) | | test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | *fileName | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:43:17:43:24 | scanf output argument | user input (value read by scanf) | -| test.c:57:10:57:16 | access to array | test.c:8:27:8:30 | **argv | test.c:57:10:57:16 | *access to array | This argument to a file access function is derived from $@ and then passed to read(fileName), which calls fopen(filename). | test.c:8:27:8:30 | **argv | user input (a command-line argument) | +| test.c:49:11:49:17 | tainted | test.c:48:21:48:26 | *call to getenv | test.c:49:11:49:17 | *tainted | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:48:21:48:26 | *call to getenv | user input (an environment variable) | +| test.c:55:11:55:16 | buffer | test.c:54:21:54:26 | *call to getenv | test.c:55:11:55:16 | *buffer | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:54:21:54:26 | *call to getenv | user input (an environment variable) | +| test.c:69:14:69:20 | access to array | test.c:8:27:8:30 | **argv | test.c:69:14:69:20 | *access to array | This argument to a file access function is derived from $@ and then passed to readFile(fileName), which calls fopen(filename). | test.c:8:27:8:30 | **argv | user input (a command-line argument) | +| test.c:76:11:76:16 | buffer | test.c:74:13:74:18 | read output argument | test.c:76:11:76:16 | *buffer | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:74:13:74:18 | read output argument | user input (buffer read by read) | +| test.c:76:11:76:16 | buffer | test.c:75:13:75:18 | read output argument | test.c:76:11:76:16 | *buffer | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:75:13:75:18 | read output argument | user input (buffer read by read) | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h index 5b6483480f73..53344da57d6d 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h @@ -6,6 +6,7 @@ typedef struct {} FILE; #define FILENAME_MAX 1000 typedef unsigned long size_t; +typedef signed long ssize_t; FILE *fopen(const char *filename, const char *mode); int sprintf(char *s, const char *format, ...); @@ -15,3 +16,4 @@ int scanf(const char *format, ...); void *malloc(size_t size); double strtod(const char *ptr, char **endptr); char *getenv(const char *name); +ssize_t read(int fd, void *buffer, size_t count); diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c index 824db8f16ada..4c4782758324 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c @@ -7,7 +7,7 @@ int main(int argc, char** argv) { char *userAndFile = argv[2]; - + { char fileBuffer[FILENAME_MAX] = "/home/"; char *fileName = fileBuffer; @@ -44,6 +44,18 @@ int main(int argc, char** argv) { fopen(fileName, "wb+"); // BAD } + { + char *tainted = getenv("A_STRING"); + fopen(tainted, "wb+"); // BAD + } + + { + char buffer[1024]; + strncpy(buffer, getenv("A_STRING"), 1024); + fopen(buffer, "wb+"); // BAD + fopen(buffer, "wb+"); // (we don't want a duplicate result here) + } + { char *aNumber = getenv("A_NUMBER"); double number = strtod(aNumber, 0); @@ -53,11 +65,18 @@ int main(int argc, char** argv) { } { - void read(const char *fileName); - read(argv[1]); // BAD + void readFile(const char *fileName); + readFile(argv[1]); // BAD + } + + { + char buffer[1024]; + read(0, buffer, 1024); + read(0, buffer, 1024); + fopen(buffer, "wb+"); // BAD [duplicated with both sources] } } -void read(char *fileName) { +void readFile(char *fileName) { fopen(fileName, "wb+"); }