From 1db759cc4d09d71d734151a7948f5cebca06b35b Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 14 Mar 2022 09:33:08 +0300 Subject: [PATCH 1/3] Update InsecureTemporaryFile.ql --- .../CWE/CWE-377/InsecureTemporaryFile.ql | 74 ------------------- 1 file changed, 74 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql b/cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql index 877b67cbbf84..0852cb90918b 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql @@ -13,30 +13,6 @@ import cpp import semmle.code.cpp.valuenumbering.GlobalValueNumbering -/** Holds for a function `f` that has an argument at index `apos` used to read the file. */ -predicate numberArgumentRead(Function f, int apos) { - f.hasGlobalOrStdName("fgets") and apos = 2 - or - f.hasGlobalOrStdName("fread") and apos = 3 - or - f.hasGlobalOrStdName("read") and apos = 0 - or - f.hasGlobalOrStdName("fscanf") and apos = 0 -} - -/** Holds for a function `f` that has an argument at index `apos` used to write to file */ -predicate numberArgumentWrite(Function f, int apos) { - f.hasGlobalOrStdName("fprintf") and apos = 0 - or - f.hasGlobalOrStdName("fputs") and apos = 1 - or - f.hasGlobalOrStdName("write") and apos = 0 - or - f.hasGlobalOrStdName("fwrite") and apos = 3 - or - f.hasGlobalOrStdName("fflush") and apos = 0 -} - from FunctionCall fc, string msg where // search for functions for generating a name, without a guarantee of the absence of a file during the period of work with it. @@ -59,54 +35,4 @@ where ) and msg = "Finding the name of a file that does not exist does not mean that it will not be exist at the next operation." - or - // finding places to work with a file without setting permissions, but with predictable names. - ( - fc.getTarget().hasGlobalOrStdName("fopen") or - fc.getTarget().hasGlobalOrStdName("open") - ) and - fc.getNumberOfArguments() = 2 and - exists(FunctionCall fctmp, int i | - numberArgumentWrite(fctmp.getTarget(), i) and - globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i)) - ) and - not exists(FunctionCall fctmp, int i | - numberArgumentRead(fctmp.getTarget(), i) and - globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i)) - ) and - exists(FunctionCall fctmp | - ( - fctmp.getTarget().hasGlobalOrStdName("strcat") or - fctmp.getTarget().hasGlobalOrStdName("strcpy") - ) and - globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getAnArgument()) - or - fctmp.getTarget().hasGlobalOrStdName("getenv") and - globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp) - or - ( - fctmp.getTarget().hasGlobalOrStdName("asprintf") or - fctmp.getTarget().hasGlobalOrStdName("vasprintf") or - fctmp.getTarget().hasGlobalOrStdName("xasprintf") or - fctmp.getTarget().hasGlobalOrStdName("xvasprintf ") - ) and - exists(Variable vrtmp | - vrtmp = fc.getArgument(0).(VariableAccess).getTarget() and - vrtmp = fctmp.getArgument(0).(AddressOfExpr).getAddressable() and - not vrtmp instanceof Field - ) - ) and - not exists(FunctionCall fctmp | - ( - fctmp.getTarget().hasGlobalOrStdName("umask") or - fctmp.getTarget().hasGlobalOrStdName("fchmod") or - fctmp.getTarget().hasGlobalOrStdName("chmod") - ) and - ( - fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or - fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock() - ) - ) and - msg = - "Creating a file for writing without evaluating its existence and setting permissions can be unsafe." select fc, msg From de92356c882a6c12ef76e4f9e1762e632c2fca42 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 14 Mar 2022 09:35:03 +0300 Subject: [PATCH 2/3] Update InsecureTemporaryFile.expected --- .../CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected index b0632edcd2a2..545b301922a3 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected @@ -1,2 +1 @@ | test.cpp:16:20:16:25 | call to tmpnam | Finding the name of a file that does not exist does not mean that it will not be exist at the next operation. | -| test.cpp:42:8:42:12 | call to fopen | Creating a file for writing without evaluating its existence and setting permissions can be unsafe. | From 62381d0762ad978d808fb1b8119415a07e028e28 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 14 Mar 2022 09:36:28 +0300 Subject: [PATCH 3/3] Update test.cpp --- .../query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp index ceffdf11fa9e..07efea49e784 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp @@ -39,7 +39,7 @@ int funcTest3() FILE *fp; char filename[80]; strcat(filename, "/tmp/tmp.name"); - fp = fopen(filename,"w"); // BAD + fp = fopen(filename,"w"); // BAD [NOT DETECTED] fprintf(fp,"%s\n","data to file"); fclose(fp); return 0;