CPP: Exclude incorrect scanf checks from missing scanf checks

Author: alexet

cpp/ql/src/Critical/IncorrectCheckScanf.ql

@@ -15,29 +15,8 @@
 
 import cpp
 import semmle.code.cpp.commons.Scanf
-
-predicate exprInBooleanContext(Expr e) {
-  e.getParent() instanceof BinaryLogicalOperation
-  or
-  e.getParent() instanceof UnaryLogicalOperation
-  or
-  e = any(IfStmt ifStmt).getCondition()
-  or
-  e = any(WhileStmt whileStmt).getCondition()
-  or
-  exists(EqualityOperation eqOp, Expr other |
-    eqOp.hasOperands(e, other) and
-    other.getValue() = "0"
-  )
-  or
-  exists(Variable v |
-    v.getAnAssignedValue() = e and
-    forex(Expr use | use = v.getAnAccess() | exprInBooleanContext(use))
-  )
-}
+import ScanfChecks
 
 from ScanfFunctionCall call
-where
-  exprInBooleanContext(call) and
-  not exists(call.getTarget().getDefinition()) // ignore calls where the scanf is defined as they might be different (e.g. linux)
+where incorrectlyCheckedScanf(call)
 select call, "The result of scanf is onyl checkeck against 0, but it can also return EOF."

cpp/ql/src/Critical/MissingCheckScanf.ql

@@ -19,6 +19,7 @@ import semmle.code.cpp.controlflow.Guards
 import semmle.code.cpp.dataflow.new.DataFlow::DataFlow
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.ir.ValueNumbering
+import ScanfChecks
 
 /** Holds if `n` reaches an argument  to a call to a `scanf`-like function. */
 pragma[nomagic]
@@ -60,7 +61,9 @@ predicate isSink(ScanfFunctionCall call, int index, Node n, Expr input) {
  * argument that has not been previously initialized.
  */
 predicate isRelevantScanfCall(ScanfFunctionCall call, int index, Expr output) {
-  exists(Node n | fwdFlow0(n) and isSink(call, index, n, output))
+  exists(Node n | fwdFlow0(n) and isSink(call, index, n, output)) and
+  // Exclude results from incorrectky checked scanf query
+  not incorrectlyCheckedScanf(call)
 }
 
 /**

cpp/ql/src/Critical/ScanfChecks.qll

@@ -0,0 +1,34 @@
+private import cpp
+private import semmle.code.cpp.commons.Scanf
+
+private predicate exprInBooleanContext(Expr e) {
+  e.getParent() instanceof BinaryLogicalOperation
+  or
+  e.getParent() instanceof UnaryLogicalOperation
+  or
+  e = any(IfStmt ifStmt).getCondition()
+  or
+  e = any(WhileStmt whileStmt).getCondition()
+  or
+  exists(EqualityOperation eqOp, Expr other |
+    eqOp.hasOperands(e, other) and
+    other.getValue() = "0"
+  )
+  or
+  exists(Variable v |
+    v.getAnAssignedValue() = e and
+    forex(Expr use | use = v.getAnAccess() | exprInBooleanContext(use))
+  )
+}
+
+private predicate isLinuxKernel() {
+  exists(Macro macro | macro.getName() = "_LINUX_KERNEL_SPRINTF_H_")
+}
+
+/**
+ * Holds if `call` is a `scanf`-like call were the result is only checked against 0, but it can also return EOF.
+ */
+predicate incorrectlyCheckedScanf(ScanfFunctionCall call) {
+  exprInBooleanContext(call) and
+  not isLinuxKernel() // scanf in the linux kernel can't return EOF
+}

cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected

@@ -5,9 +5,6 @@
 | test.cpp:98:7:98:8 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:97:3:97:7 | call to scanf | call to scanf |
 | test.cpp:108:7:108:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:107:3:107:8 | call to fscanf | call to fscanf |
 | test.cpp:115:7:115:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:114:3:114:8 | call to sscanf | call to sscanf |
-| test.cpp:164:8:164:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:162:7:162:11 | call to scanf | call to scanf |
-| test.cpp:173:8:173:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:171:7:171:11 | call to scanf | call to scanf |
-| test.cpp:205:8:205:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:204:7:204:11 | call to scanf | call to scanf |
 | test.cpp:224:8:224:8 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:221:7:221:11 | call to scanf | call to scanf |
 | test.cpp:248:9:248:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:246:25:246:29 | call to scanf | call to scanf |
 | test.cpp:252:9:252:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:250:14:250:18 | call to scanf | call to scanf |