From bddb5fd9f9ff7d6615f48d162c1f9c290e637493 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Fri, 25 Feb 2022 11:14:20 +0300 Subject: [PATCH 01/21] Add files via upload --- .../CWE-754/ImproperCheckReturnValueScanf.cpp | 10 ++ .../ImproperCheckReturnValueScanf.qhelp | 22 ++++ .../CWE-754/ImproperCheckReturnValueScanf.ql | 108 ++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp new file mode 100644 index 000000000000..432f144b4bb2 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp @@ -0,0 +1,10 @@ +... + r = scanf("%i", i); + if (r == 1) // GOOD + return i; + else + return -1; +... + scanf("%i", i); // BAD + return i; +... diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp new file mode 100644 index 000000000000..2e60d33f89ab --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp @@ -0,0 +1,22 @@ + + + +

Working with reading data without validation procedures and with uninitialized arguments can lead to unpredictable consequences.

+ + + +

The following example demonstrates erroneous and corrected work with a function call.

+ + + + + +
  • + CERT C Coding Standard: + EXP12-C. Do not ignore values returned by functions. +
    • + +     +     diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql new file mode 100644 index 000000000000..4dc141be2474 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql @@ -0,0 +1,108 @@ +/** + * @name Improper check return value scanf. + * @description Using a function call without the ability to evaluate the correctness of the work can lead to unexpected results. + * @kind problem + * @id cpp/improper-check-return-value-scanf + * @problem.severity warning + * @precision medium + * @tags correctness + * security + * external/cwe/cwe-754 + * external/cwe/cwe-908 + */ + +import cpp +import semmle.code.cpp.commons.Exclusions +import semmle.code.cpp.valuenumbering.GlobalValueNumbering + +/** Returns the starting position of the argument being filled. */ +int posArgumentInFunctionCall(FunctionCall fc) { + ( + ( + fc.getTarget().hasGlobalOrStdName("scanf") or + fc.getTarget().hasGlobalOrStdName("scanf_s") + ) and + result = 1 + or + ( + fc.getTarget().hasGlobalOrStdName("fscanf") or + fc.getTarget().hasGlobalOrStdName("sscanf") or + fc.getTarget().hasGlobalOrStdName("fscanf_s") or + fc.getTarget().hasGlobalOrStdName("sscanf_s") + ) and + result = 2 + ) +} + +from FunctionCall fc, int n +where + n = posArgumentInFunctionCall(fc) and + // Function return value is not evaluated. + fc instanceof ExprInVoidContext and + not isFromMacroDefinition(fc) and + exists(Expr e0, int i | + i in [n .. fc.getNumberOfArguments() - 1] and + // Fillable argument was not initialized. + ( + fc.getArgument(i).(VariableAccess).getTarget() instanceof LocalScopeVariable or + fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() instanceof + LocalScopeVariable + ) and + ( + not fc.getArgument(i).(VariableAccess).getTarget().hasInitializer() and + not fc.getArgument(i) + .(AddressOfExpr) + .getOperand() + .(VariableAccess) + .getTarget() + .hasInitializer() + ) and + ( + not fc.getArgument(i).(VariableAccess).getTarget().getAnAssignment().getASuccessor+() = fc and + not fc.getArgument(i) + .(AddressOfExpr) + .getOperand() + .(VariableAccess) + .getTarget() + .getAnAssignment() + .getASuccessor+() = fc + ) and + // After the call, the completed arguments are assigned or returned as the result of the operation of the upper function. + fc.getASuccessor+() = e0 and + ( + ( + e0.(Assignment).getRValue().(VariableAccess).getTarget() = + fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or + e0.(Assignment).getRValue().(VariableAccess).getTarget() = + fc.getArgument(i).(VariableAccess).getTarget() + ) + or + e0.getEnclosingStmt() instanceof ReturnStmt and + ( + e0.(VariableAccess).getTarget() = + fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or + e0.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() + ) + or + not exists(Expr e1 | + fc.getASuccessor+() = e1 and + ( + e1.(VariableAccess).getTarget() = + fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or + e1.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() + ) + ) + ) + ) and + // After the call, filled arguments are not evaluated. + not exists(Expr e0, int i | + i in [n .. fc.getNumberOfArguments() - 1] and + fc.getASuccessor+() = e0 and + e0.getEnclosingElement() instanceof ComparisonOperation and + ( + e0.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() or + e0.(VariableAccess).getTarget() = + fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() + ) + ) +select fc, "Unchecked return value for call to '" + fc.getTarget().getName() + "'." From 0c8a07218c9c201e376f72e0547399773604fd97 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Fri, 25 Feb 2022 11:16:05 +0300 Subject: [PATCH 02/21] Add files via upload --- .../ImproperCheckReturnValueScanf.expected | 2 + .../tests/ImproperCheckReturnValueScanf.qlref | 1 + .../CWE/CWE-754/semmle/tests/test.cpp | 53 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.qlref create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected new file mode 100644 index 000000000000..7ba0580ca901 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected @@ -0,0 +1,2 @@ +| test.cpp:23:3:23:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:41:3:41:7 | call to scanf | Unchecked return value for call to 'scanf'. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.qlref new file mode 100644 index 000000000000..f0cb9dd57c1e --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp new file mode 100644 index 000000000000..4389cb506a2e --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -0,0 +1,53 @@ +int scanf(const char *format, ...); +int globalVal; +int functionWork1() { + int i; + if (scanf("%i", i) == 1) // GOOD + return i; + else + return -1; +} + +int functionWork1_() { + int i; + int r; + r = scanf("%i", i); + if (r == 1) // GOOD + return i; + else + return -1; +} + +int functionWork1b() { + int i; + scanf("%i", i); // BAD + return i; +} + +int functionWork2() { + int i = 0; + scanf("%i", i); // GOOD:the error can be determined by examining the initial value. + return i; +} + +int functionWork2_() { + int i; + i = 0; + scanf("%i", i); // GOOD:the error can be determined by examining the initial value. + return i; +} +int functionWork2b() { + int i; + scanf("%i", i); // BAD + globalVal = i; + return 0; +} + +void functionRunner() { + functionWork1(); + functionWork1_(); + functionWork1b(); + functionWork2(); + functionWork2_(); + functionWork2b(); +} From d772ea0efe81f9f4917f84d6f95b7eed40cc222b Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 1 Mar 2022 10:49:36 +0300 Subject: [PATCH 03/21] Apply suggestions from code review Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../CWE/CWE-754/ImproperCheckReturnValueScanf.cpp | 4 ++-- .../CWE/CWE-754/ImproperCheckReturnValueScanf.ql | 12 ++++-------- .../Security/CWE/CWE-754/semmle/tests/test.cpp | 12 ++++++------ 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp index 432f144b4bb2..4928db37a178 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.cpp @@ -1,10 +1,10 @@ ... - r = scanf("%i", i); + r = scanf("%i", &i); if (r == 1) // GOOD return i; else return -1; ... - scanf("%i", i); // BAD + scanf("%i", &i); // BAD return i; ... diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql index 4dc141be2474..67eebb62bb9e 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql @@ -1,5 +1,5 @@ /** - * @name Improper check return value scanf. + * @name Improper check return value scanf * @description Using a function call without the ability to evaluate the correctness of the work can lead to unexpected results. * @kind problem * @id cpp/improper-check-return-value-scanf @@ -15,20 +15,16 @@ import cpp import semmle.code.cpp.commons.Exclusions import semmle.code.cpp.valuenumbering.GlobalValueNumbering -/** Returns the starting position of the argument being filled. */ +/** Returns the position of the first argument being filled. */ int posArgumentInFunctionCall(FunctionCall fc) { ( ( - fc.getTarget().hasGlobalOrStdName("scanf") or - fc.getTarget().hasGlobalOrStdName("scanf_s") + fc.getTarget().hasGlobalOrStdName(["scanf", "scanf_s"]) ) and result = 1 or ( - fc.getTarget().hasGlobalOrStdName("fscanf") or - fc.getTarget().hasGlobalOrStdName("sscanf") or - fc.getTarget().hasGlobalOrStdName("fscanf_s") or - fc.getTarget().hasGlobalOrStdName("sscanf_s") + fc.getTarget().hasGlobalOrStdName(["fscanf", "sscanf", "fscanf_s", "sscanf_s"]) ) and result = 2 ) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 4389cb506a2e..588100e5b786 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -2,7 +2,7 @@ int scanf(const char *format, ...); int globalVal; int functionWork1() { int i; - if (scanf("%i", i) == 1) // GOOD + if (scanf("%i", &i) == 1) // GOOD return i; else return -1; @@ -11,7 +11,7 @@ int functionWork1() { int functionWork1_() { int i; int r; - r = scanf("%i", i); + r = scanf("%i", &i); if (r == 1) // GOOD return i; else @@ -20,25 +20,25 @@ int functionWork1_() { int functionWork1b() { int i; - scanf("%i", i); // BAD + scanf("%i", &i); // BAD return i; } int functionWork2() { int i = 0; - scanf("%i", i); // GOOD:the error can be determined by examining the initial value. + scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. return i; } int functionWork2_() { int i; i = 0; - scanf("%i", i); // GOOD:the error can be determined by examining the initial value. + scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. return i; } int functionWork2b() { int i; - scanf("%i", i); // BAD + scanf("%i", &i); // BAD globalVal = i; return 0; } From bfec3c5e6ebd99db051205d8c83c0371c50447a1 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 1 Mar 2022 16:35:31 +0300 Subject: [PATCH 04/21] Update ImproperCheckReturnValueScanf.expected --- .../semmle/tests/ImproperCheckReturnValueScanf.expected | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected index 7ba0580ca901..abea6ce76399 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected @@ -1,2 +1,7 @@ -| test.cpp:23:3:23:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:41:3:41:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:43:3:43:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:44:3:44:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:45:3:45:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:78:3:78:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:79:3:79:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:80:3:80:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:88:3:88:7 | call to scanf | Unchecked return value for call to 'scanf'. | From e9fefab9b150bc33bff313b91be50211ba02d90d Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 1 Mar 2022 16:36:24 +0300 Subject: [PATCH 05/21] Update test.cpp --- .../CWE/CWE-754/semmle/tests/test.cpp | 58 +++++++++++++++++-- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 588100e5b786..e3a464ed6f5e 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -1,45 +1,92 @@ int scanf(const char *format, ...); int globalVal; +char * globalVala; +int * globalValp; +char globalVala2; int functionWork1() { int i; - if (scanf("%i", &i) == 1) // GOOD - return i; - else + char a[10]; + int b; + int *p = &b; + if (scanf("%i", &i) != 1) // GOOD return -1; + if (scanf("%s", a) != 1) // GOOD + return -1; + if (scanf("%i", p) != 1) // GOOD + return -1; + return i; } int functionWork1_() { int i; + char a[10]; + int b; + int *p = &b; int r; r = scanf("%i", &i); + if (r != 1) // GOOD + return -1; + r = scanf("%s", a); if (r == 1) // GOOD - return i; - else return -1; + r = scanf("%i", p); + if (r != 1) // GOOD + return -1; + return i; } int functionWork1b() { int i; + char a[10]; + int b; + int *p = &b; scanf("%i", &i); // BAD + scanf("%s", a); // BAD + scanf("%i", p); // BAD return i; } int functionWork2() { int i = 0; + char a[10] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + int b = 1; + int *p = &b; scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. + scanf("%s", a); // GOOD:the error can be determined by examining the initial value. + scanf("%i", p); // GOOD:the error can be determined by examining the initial value. return i; } int functionWork2_() { int i; i = 0; + char a[10]; + a[0] = 0; + int b; + b=1; + int *p = &b; scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. + scanf("%s", a); // GOOD:the error can be determined by examining the initial value. + scanf("%i", p); // GOOD:the error can be determined by examining the initial value. return i; } int functionWork2b() { int i; + char a[10]; + int b; + int *p = &b; scanf("%i", &i); // BAD + scanf("%s", a); // BAD + scanf("%i", p); // BAD globalVal = i; + globalVala = a; + globalValp = p; + return 0; +} +int functionWork2b_() { + char a[10]; + scanf("%s", a); // BAD + globalVala2 = a[0]; return 0; } @@ -50,4 +97,5 @@ void functionRunner() { functionWork2(); functionWork2_(); functionWork2b(); + functionWork2b_(); } From a6654fce4acc64503f993046abfca5c4c44ece39 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 1 Mar 2022 16:37:29 +0300 Subject: [PATCH 06/21] Update ImproperCheckReturnValueScanf.ql --- .../CWE-754/ImproperCheckReturnValueScanf.ql | 105 +++++++++--------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql index 67eebb62bb9e..1716632cab89 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql @@ -13,92 +13,91 @@ import cpp import semmle.code.cpp.commons.Exclusions -import semmle.code.cpp.valuenumbering.GlobalValueNumbering /** Returns the position of the first argument being filled. */ int posArgumentInFunctionCall(FunctionCall fc) { ( - ( - fc.getTarget().hasGlobalOrStdName(["scanf", "scanf_s"]) - ) and + fc.getTarget().hasGlobalOrStdName(["scanf", "scanf_s"]) and result = 1 or - ( - fc.getTarget().hasGlobalOrStdName(["fscanf", "sscanf", "fscanf_s", "sscanf_s"]) - ) and + fc.getTarget().hasGlobalOrStdName(["fscanf", "sscanf", "fscanf_s", "sscanf_s"]) and result = 2 ) } -from FunctionCall fc, int n -where - n = posArgumentInFunctionCall(fc) and - // Function return value is not evaluated. - fc instanceof ExprInVoidContext and - not isFromMacroDefinition(fc) and - exists(Expr e0, int i | - i in [n .. fc.getNumberOfArguments() - 1] and +/** Holds if a function argument was not initialized but used after the call. */ +predicate argumentIsNotInitializedAndIsUsed(Variable vt, FunctionCall fc) { + exists(Expr e0 | // Fillable argument was not initialized. + vt instanceof LocalScopeVariable and + not vt.getAnAssignment().getASuccessor+() = fc and ( - fc.getArgument(i).(VariableAccess).getTarget() instanceof LocalScopeVariable or - fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() instanceof - LocalScopeVariable + not vt.hasInitializer() + or + exists(Expr e1, Variable v1 | + e1 = vt.getInitializer().getExpr() and + v1 = e1.(AddressOfExpr).getOperand().(VariableAccess).getTarget() and + ( + not v1.hasInitializer() and + not v1.getAnAssignment().getASuccessor+() = fc + ) + ) ) and - ( - not fc.getArgument(i).(VariableAccess).getTarget().hasInitializer() and - not fc.getArgument(i) - .(AddressOfExpr) - .getOperand() - .(VariableAccess) - .getTarget() - .hasInitializer() + not exists(AssignExpr ae | + ae.getLValue() = vt.getAnAccess().getParent() and + ae.getASuccessor+() = fc ) and - ( - not fc.getArgument(i).(VariableAccess).getTarget().getAnAssignment().getASuccessor+() = fc and - not fc.getArgument(i) - .(AddressOfExpr) - .getOperand() - .(VariableAccess) - .getTarget() - .getAnAssignment() - .getASuccessor+() = fc + not exists(FunctionCall f0 | + f0.getAnArgument().getAChild() = vt.getAnAccess() and + f0.getASuccessor+() = fc ) and // After the call, the completed arguments are assigned or returned as the result of the operation of the upper function. fc.getASuccessor+() = e0 and ( ( - e0.(Assignment).getRValue().(VariableAccess).getTarget() = - fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or - e0.(Assignment).getRValue().(VariableAccess).getTarget() = - fc.getArgument(i).(VariableAccess).getTarget() + e0.(Assignment).getRValue().(VariableAccess).getTarget() = vt or + e0.(Assignment).getRValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = vt ) or e0.getEnclosingStmt() instanceof ReturnStmt and - ( - e0.(VariableAccess).getTarget() = - fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or - e0.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() - ) + e0.(VariableAccess).getTarget() = vt or not exists(Expr e1 | fc.getASuccessor+() = e1 and - ( - e1.(VariableAccess).getTarget() = - fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() or - e1.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() - ) + e1.(VariableAccess).getTarget() = vt ) ) + ) +} + +from FunctionCall fc, int i +where + // Function return value is not evaluated. + fc instanceof ExprInVoidContext and + not isFromMacroDefinition(fc) and + i in [posArgumentInFunctionCall(fc) .. fc.getNumberOfArguments() - 1] and + ( + argumentIsNotInitializedAndIsUsed(fc.getArgument(i).(VariableAccess).getTarget(), fc) or + argumentIsNotInitializedAndIsUsed(fc.getArgument(i) + .(AddressOfExpr) + .getOperand() + .(VariableAccess) + .getTarget(), fc) or + argumentIsNotInitializedAndIsUsed(fc.getArgument(i) + .(ArrayExpr) + .getArrayBase() + .(VariableAccess) + .getTarget(), fc) ) and // After the call, filled arguments are not evaluated. - not exists(Expr e0, int i | - i in [n .. fc.getNumberOfArguments() - 1] and + not exists(Expr e0, int i1 | + i1 in [posArgumentInFunctionCall(fc) .. fc.getNumberOfArguments() - 1] and fc.getASuccessor+() = e0 and e0.getEnclosingElement() instanceof ComparisonOperation and ( - e0.(VariableAccess).getTarget() = fc.getArgument(i).(VariableAccess).getTarget() or + e0.(VariableAccess).getTarget() = fc.getArgument(i1).(VariableAccess).getTarget() or e0.(VariableAccess).getTarget() = - fc.getArgument(i).(AddressOfExpr).getOperand().(VariableAccess).getTarget() + fc.getArgument(i1).(AddressOfExpr).getOperand().(VariableAccess).getTarget() ) ) select fc, "Unchecked return value for call to '" + fc.getTarget().getName() + "'." From 1a30b8d467b105dffab76ba9e19a0f1461e1d892 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:14:22 +0300 Subject: [PATCH 07/21] Apply suggestions from code review Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index e3a464ed6f5e..9b4d78e9f25e 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -48,7 +48,7 @@ int functionWork1b() { int functionWork2() { int i = 0; - char a[10] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + char a[10] = ""; int b = 1; int *p = &b; scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. @@ -61,7 +61,7 @@ int functionWork2_() { int i; i = 0; char a[10]; - a[0] = 0; + a[0] = '\0'; int b; b=1; int *p = &b; From 547342cd61a336d911075f7cd0847b81b2296334 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:16:00 +0300 Subject: [PATCH 08/21] Update test.cpp --- .../Security/CWE/CWE-754/semmle/tests/test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 9b4d78e9f25e..0181915b4e0f 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -51,9 +51,9 @@ int functionWork2() { char a[10] = ""; int b = 1; int *p = &b; - scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. - scanf("%s", a); // GOOD:the error can be determined by examining the initial value. - scanf("%i", p); // GOOD:the error can be determined by examining the initial value. + scanf("%i", &i); // GOOD:Argument initialized even when scanf fails. + scanf("%s", a); // GOOD:Argument initialized even when scanf fails. + scanf("%i", p); // GOOD:Argument initialized even when scanf fails. return i; } @@ -65,9 +65,9 @@ int functionWork2_() { int b; b=1; int *p = &b; - scanf("%i", &i); // GOOD:the error can be determined by examining the initial value. - scanf("%s", a); // GOOD:the error can be determined by examining the initial value. - scanf("%i", p); // GOOD:the error can be determined by examining the initial value. + scanf("%i", &i); // GOOD:Argument initialized even when scanf fails. + scanf("%s", a); // GOOD:Argument initialized even when scanf fails. + scanf("%i", p); // GOOD:Argument initialized even when scanf fails. return i; } int functionWork2b() { From 2dc85e183c56cf381c3f1bc03b589c4ea04bd3f0 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:20:41 +0300 Subject: [PATCH 09/21] Update test.cpp --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 0181915b4e0f..e9f05e4cd965 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -35,7 +35,7 @@ int functionWork1_() { return i; } -int functionWork1b() { +int functionWork1b(int retIndex) { int i; char a[10]; int b; @@ -43,6 +43,10 @@ int functionWork1b() { scanf("%i", &i); // BAD scanf("%s", a); // BAD scanf("%i", p); // BAD + if(retIndex == 0) + return (int)a; + if(retIndex == 1) + return *p; return i; } From 25b3aba8236a67535044b72f47e8215347b44372 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:21:38 +0300 Subject: [PATCH 10/21] Update test.cpp --- .../Security/CWE/CWE-754/semmle/tests/test.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index e9f05e4cd965..227df8fe731d 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -93,13 +93,3 @@ int functionWork2b_() { globalVala2 = a[0]; return 0; } - -void functionRunner() { - functionWork1(); - functionWork1_(); - functionWork1b(); - functionWork2(); - functionWork2_(); - functionWork2b(); - functionWork2b_(); -} From 8e0c0ad200fa30a51dd4a3aee0b0410d23842cb0 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:37:31 +0300 Subject: [PATCH 11/21] Update test.cpp --- .../CWE/CWE-754/semmle/tests/test.cpp | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 227df8fe731d..14c0328050bb 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -3,7 +3,7 @@ int globalVal; char * globalVala; int * globalValp; char globalVala2; -int functionWork1() { +int functionWork1(int retIndex) { int i; char a[10]; int b; @@ -14,10 +14,14 @@ int functionWork1() { return -1; if (scanf("%i", p) != 1) // GOOD return -1; + if(retIndex == 0) + return (int)a; + if(retIndex == 1) + return *p; return i; } -int functionWork1_() { +int functionWork1_(int retIndex) { int i; char a[10]; int b; @@ -32,6 +36,10 @@ int functionWork1_() { r = scanf("%i", p); if (r != 1) // GOOD return -1; + if(retIndex == 0) + return (int)a; + if(retIndex == 1) + return *p; return i; } @@ -49,8 +57,14 @@ int functionWork1b(int retIndex) { return *p; return i; } - -int functionWork2() { +int functionWork1_() { + int i; + scanf("%i",&i); + if(i<10) + return -1; + return i; +} +int functionWork2(int retIndex) { int i = 0; char a[10] = ""; int b = 1; @@ -58,10 +72,14 @@ int functionWork2() { scanf("%i", &i); // GOOD:Argument initialized even when scanf fails. scanf("%s", a); // GOOD:Argument initialized even when scanf fails. scanf("%i", p); // GOOD:Argument initialized even when scanf fails. + if(retIndex == 0) + return (int)a; + if(retIndex == 1) + return *p; return i; } -int functionWork2_() { +int functionWork2_(int retIndex) { int i; i = 0; char a[10]; @@ -72,6 +90,10 @@ int functionWork2_() { scanf("%i", &i); // GOOD:Argument initialized even when scanf fails. scanf("%s", a); // GOOD:Argument initialized even when scanf fails. scanf("%i", p); // GOOD:Argument initialized even when scanf fails. + if(retIndex == 0) + return (int)a; + if(retIndex == 1) + return *p; return i; } int functionWork2b() { From bec4170bdfa1fba2037c8e6d125d6a83d64aa912 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:39:19 +0300 Subject: [PATCH 12/21] Update ImproperCheckReturnValueScanf.expected --- .../tests/ImproperCheckReturnValueScanf.expected | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected index abea6ce76399..1a5d816f3f18 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected @@ -1,7 +1,7 @@ -| test.cpp:43:3:43:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:44:3:44:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:45:3:45:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:78:3:78:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:79:3:79:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:80:3:80:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:88:3:88:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:51:3:51:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:52:3:52:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:53:3:53:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:104:3:104:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:105:3:105:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:106:3:106:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:114:3:114:7 | call to scanf | Unchecked return value for call to 'scanf'. | From 01f9114a80ab767d3f618dab03ef5df30d318870 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 3 Mar 2022 10:57:11 +0300 Subject: [PATCH 13/21] Update test.cpp --- .../Security/CWE/CWE-754/semmle/tests/test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 14c0328050bb..051d2e00ddc7 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -15,7 +15,7 @@ int functionWork1(int retIndex) { if (scanf("%i", p) != 1) // GOOD return -1; if(retIndex == 0) - return (int)a; + return (int)*a; if(retIndex == 1) return *p; return i; @@ -37,7 +37,7 @@ int functionWork1_(int retIndex) { if (r != 1) // GOOD return -1; if(retIndex == 0) - return (int)a; + return (int)*a; if(retIndex == 1) return *p; return i; @@ -52,14 +52,14 @@ int functionWork1b(int retIndex) { scanf("%s", a); // BAD scanf("%i", p); // BAD if(retIndex == 0) - return (int)a; + return (int)*a; if(retIndex == 1) return *p; return i; } int functionWork1_() { int i; - scanf("%i",&i); + scanf("%i",&i); // GOOD if(i<10) return -1; return i; @@ -73,7 +73,7 @@ int functionWork2(int retIndex) { scanf("%s", a); // GOOD:Argument initialized even when scanf fails. scanf("%i", p); // GOOD:Argument initialized even when scanf fails. if(retIndex == 0) - return (int)a; + return (int)*a; if(retIndex == 1) return *p; return i; @@ -91,7 +91,7 @@ int functionWork2_(int retIndex) { scanf("%s", a); // GOOD:Argument initialized even when scanf fails. scanf("%i", p); // GOOD:Argument initialized even when scanf fails. if(retIndex == 0) - return (int)a; + return (int)*a; if(retIndex == 1) return *p; return i; From c0c7748c5ed8e1bf12f99574410ed03042a0b9c8 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 8 Mar 2022 07:42:35 +0300 Subject: [PATCH 14/21] Apply suggestions from code review Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../ImproperCheckReturnValueScanf.qhelp | 5 ++ .../CWE-754/ImproperCheckReturnValueScanf.ql | 50 +++++++++---------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp index 2e60d33f89ab..b4571cce6bdb 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp @@ -4,7 +4,12 @@

    Working with reading data without validation procedures and with uninitialized arguments can lead to unpredictable consequences.

    +     + +

    +The user should check the return value of `scanf` and related functions and check that any additional argument was assigned a value before reading the additional argument. +

    The following example demonstrates erroneous and corrected work with a function call.

    diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql index 1716632cab89..5f296752c1c4 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql @@ -1,6 +1,6 @@ /** - * @name Improper check return value scanf - * @description Using a function call without the ability to evaluate the correctness of the work can lead to unexpected results. + * @name Improper check of return value of scanf + * @description Not checking the return value of scanf and related functions may lead to undefined behavior. * @kind problem * @id cpp/improper-check-return-value-scanf * @problem.severity warning @@ -27,30 +27,30 @@ int posArgumentInFunctionCall(FunctionCall fc) { /** Holds if a function argument was not initialized but used after the call. */ predicate argumentIsNotInitializedAndIsUsed(Variable vt, FunctionCall fc) { - exists(Expr e0 | - // Fillable argument was not initialized. - vt instanceof LocalScopeVariable and - not vt.getAnAssignment().getASuccessor+() = fc and - ( - not vt.hasInitializer() - or - exists(Expr e1, Variable v1 | - e1 = vt.getInitializer().getExpr() and - v1 = e1.(AddressOfExpr).getOperand().(VariableAccess).getTarget() and - ( - not v1.hasInitializer() and - not v1.getAnAssignment().getASuccessor+() = fc - ) + // Fillable argument was not initialized. + vt instanceof LocalScopeVariable and + not vt.getAnAssignment().getASuccessor+() = fc and + ( + not vt.hasInitializer() + or + exists(Expr e, Variable v | + e = vt.getInitializer().getExpr() and + v = e.(AddressOfExpr).getOperand().(VariableAccess).getTarget() and + ( + not v.hasInitializer() and + not v.getAnAssignment().getASuccessor+() = fc ) - ) and - not exists(AssignExpr ae | - ae.getLValue() = vt.getAnAccess().getParent() and - ae.getASuccessor+() = fc - ) and - not exists(FunctionCall f0 | - f0.getAnArgument().getAChild() = vt.getAnAccess() and - f0.getASuccessor+() = fc - ) and + ) + ) and + not exists(AssignExpr ae | + ae.getLValue() = vt.getAnAccess().getParent() and + ae.getASuccessor+() = fc + ) and + not exists(FunctionCall f0 | + f0.getAnArgument().getAChild() = vt.getAnAccess() and + f0.getASuccessor+() = fc + ) and + exists(Expr e0 | // After the call, the completed arguments are assigned or returned as the result of the operation of the upper function. fc.getASuccessor+() = e0 and ( From 8335778e20f4bf074000491626f2b79b63b35dc3 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Tue, 8 Mar 2022 07:45:07 +0300 Subject: [PATCH 15/21] Update ImproperCheckReturnValueScanf.qhelp --- .../Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp index b4571cce6bdb..ab40910f5d3f 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp @@ -3,7 +3,7 @@ "qhelp.dtd"> -

    Working with reading data without validation procedures and with uninitialized arguments can lead to unpredictable consequences.

    +

    The `scanf` family functions does not require the memory pointed to by its additional pointer arguments to be initialized before calling. The user is required to check the return value of `scanf` and similar functions to establish how many of the additional arguments were assigned values. Not checking the return value and reading one of the arguments not assigned a value is undefined behavior and may have unexpected consequences.

     @@ -12,7 +12,7 @@ The user should check the return value of `scanf` and related functions and chec

    -

    The following example demonstrates erroneous and corrected work with a function call.

    +

    The first first example below is correct, as value of `i` is only read once it is checked that `scanf` has read one item. The second example is incorrect, as the return value of `scanf` is not checked, and as `scanf` might have failed to read any item before returning.

     From 5e23615be7f4f9ddb1d427011379a28708bd0df4 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 10:12:29 +0300 Subject: [PATCH 16/21] Update test.cpp --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index 051d2e00ddc7..d548f8f15077 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -59,7 +59,7 @@ int functionWork1b(int retIndex) { } int functionWork1_() { int i; - scanf("%i",&i); // GOOD + scanf("%i",&i); // BAD [NOT DETECTED] if(i<10) return -1; return i; @@ -115,3 +115,7 @@ int functionWork2b_() { globalVala2 = a[0]; return 0; } +int functionWork3b(int i) { + scanf("%i", &i); // BAD + return 0; +} From 4b451cfee6b137e58bce0d9144e71e8da134c69a Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 10:13:04 +0300 Subject: [PATCH 17/21] Update ImproperCheckReturnValueScanf.expected --- .../CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected index 1a5d816f3f18..845f20149a39 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected @@ -5,3 +5,4 @@ | test.cpp:105:3:105:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:106:3:106:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:114:3:114:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:119:3:119:7 | call to scanf | Unchecked return value for call to 'scanf'. | From fa3ce61369c3963b7b9fa6635d8e0819c15724ae Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 17:54:03 +0300 Subject: [PATCH 18/21] Update test.cpp --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index d548f8f15077..d7495392bba4 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -119,3 +119,10 @@ int functionWork3b(int i) { scanf("%i", &i); // BAD return 0; } +int functionWork3() { + char number[] = "42"; + int d; + sscanf(number, "%d", &d); // GOOD: sscanf always succeeds + if (d < 16) + return -1; +} From a094e6f63b969138d5bfee0600d623a45abac7f0 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 17:56:34 +0300 Subject: [PATCH 19/21] Update test.cpp --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index d7495392bba4..dd7858655528 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -115,8 +115,8 @@ int functionWork2b_() { globalVala2 = a[0]; return 0; } -int functionWork3b(int i) { - scanf("%i", &i); // BAD +int functionWork3b(int * i) { + scanf("%i", i); // BAD return 0; } int functionWork3() { From 623f3fbe21848d6e851b608b388af8b9e8e256ab Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 21:10:41 +0300 Subject: [PATCH 20/21] Update test.cpp --- .../query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp index dd7858655528..b9608b757b9c 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp @@ -1,4 +1,5 @@ int scanf(const char *format, ...); +int sscanf(const char *str, const char *format, ...); int globalVal; char * globalVala; int * globalValp; From ac8adeabf57582b26be95a037836e294142e7d32 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 10 Mar 2022 21:12:23 +0300 Subject: [PATCH 21/21] Update ImproperCheckReturnValueScanf.expected --- .../semmle/tests/ImproperCheckReturnValueScanf.expected | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected index 845f20149a39..85a929ed39c9 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/ImproperCheckReturnValueScanf.expected @@ -1,8 +1,8 @@ -| test.cpp:51:3:51:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:52:3:52:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:53:3:53:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:104:3:104:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:54:3:54:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:105:3:105:7 | call to scanf | Unchecked return value for call to 'scanf'. | | test.cpp:106:3:106:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:114:3:114:7 | call to scanf | Unchecked return value for call to 'scanf'. | -| test.cpp:119:3:119:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:107:3:107:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:115:3:115:7 | call to scanf | Unchecked return value for call to 'scanf'. | +| test.cpp:120:3:120:7 | call to scanf | Unchecked return value for call to 'scanf'. |