github · d10c · Aug 11, 2022 · Jul 28, 2022 · Jul 28, 2022 · Aug 11, 2022
diff --git a/.gitignore b/.gitignore
@@ -15,6 +15,9 @@
 .vs/*
 !.vs/VSWorkspaceSettings.json
 
+# VSCode temporaries
+.vscode/*.log
+
 # Byte-compiled python files
 *.pyc
 

@@ -0,0 +1,122 @@
+/**
+ * @name Missing return-value check for a scanf-like function
+ * @description TODO
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity TODO
+ * @precision TODO
+ * @id cpp/missing-check-scanf
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.commons.Scanf
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.ValueNumbering
+
+/** An expression appearing as an output argument to a `scanf`-like call */
+class ScanfOutput extends Expr {
+  ScanfFunctionCall call;
+  int varargIndex;
+  Instruction instr;
+  ValueNumber valNum;
+
+  ScanfOutput() {
+    this = call.getArgument(call.getTarget().getNumberOfParameters() + varargIndex) and
+    varargIndex >= 0 and
+    instr.getUnconvertedResultExpression() = this and
+    valueNumber(instr) = valNum and
+    // The following line is a kludge to prohibit more than one associated `instr` field,
+    // as would occur, for example, when `this` is an access to an array variable.
+    not instr instanceof ConvertInstruction
+  }
+
+  ScanfFunctionCall getCall() { result = call }
+
+  /**
+   * Any subsequent use of this argument should be surrounded by a
+   * check ensuring that the `scanf`-like function has returned a value
+   * equal to at least `getMinimumGuardConstant()`.
+   */
+  int getMinimumGuardConstant() {
+    result =
+      varargIndex + 1 -
+        count(ScanfFormatLiteral f, int n |
+          n <= varargIndex and f.getUse() = call and f.parseConvSpec(n, _, _, _, "n")
+        )
+  }
+
+  predicate hasGuardedAccess(Access e, boolean isGuarded) {
+    e = this.getAnAccess() and
+    if
+      exists(int value, int minGuard | minGuard = this.getMinimumGuardConstant() |
+        e.getBasicBlock() = blockGuardedBy(value, "==", call) and minGuard <= value
+        or
+        e.getBasicBlock() = blockGuardedBy(value, "<", call) and minGuard - 1 <= value
+        or
+        e.getBasicBlock() = blockGuardedBy(value, "<=", call) and minGuard <= value
+      )
+    then isGuarded = true
+    else isGuarded = false
+  }
+
+  /**
+   * Get a subsequent access of the same underlying storage,
+   * but before it gets reset or reused in another `scanf` call.
+   */
+  Access getAnAccess() {
+    exists(Instruction dst |
+      this.bigStep(instr) = dst and
+      dst.getAst() = result and
+      valueNumber(dst) = valNum
+    )
+  }
+
+  private Instruction bigStep(Instruction i) {
+    result = this.smallStep(i)
+    or
+    exists(Instruction j | j = this.bigStep(i) | result = this.smallStep(j))
+  }
+
+  private Instruction smallStep(Instruction i) {
+    instr.getASuccessor*() = i and
+    i.getASuccessor() = result and
+    not this.isBarrier(result)
+  }
+
+  private predicate isBarrier(Instruction i) {
+    valueNumber(i) = valNum and
+    exists(Expr e | i.getAst() = e |
+      i = any(StoreInstruction s).getDestinationAddress()
+      or
+      [e, e.getParent().(AddressOfExpr)] instanceof ScanfOutput
+    )
+  }
+}
+
+/** Returns a block guarded by the assertion of $value $op $call */
+BasicBlock blockGuardedBy(int value, string op, ScanfFunctionCall call) {
+  exists(GuardCondition g, Expr left, Expr right |
+    right = g.getAChild() and
+    value = left.getValue().toInt() and
+    DataFlow::localExprFlow(call, right)
+  |
+    g.ensuresEq(left, right, 0, result, true) and op = "=="
+    or
+    g.ensuresLt(left, right, 0, result, true) and op = "<"
+    or
+    g.ensuresLt(left, right, 1, result, true) and op = "<="
+  )
+}
+
+from ScanfOutput output, ScanfFunctionCall call, ScanfFunction fun, Access access
+where
+  call.getTarget() = fun and
+  output.getCall() = call and
+  output.hasGuardedAccess(access, false)
+select access,
+  "$@ is read here, but may not have been written. " +
+    "It should be guarded by a check that $@() returns at least " + output.getMinimumGuardConstant()
+    + ".", access, access.toString(), call, fun.getName()
@@ -0,0 +1 @@
+DisableFormat: true
@@ -0,0 +1,19 @@
+| test.cpp:30:7:30:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:30:7:30:7 | i | i | test.cpp:29:3:29:7 | call to scanf | scanf |
+| test.cpp:46:7:46:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:46:7:46:7 | i | i | test.cpp:45:3:45:7 | call to scanf | scanf |
+| test.cpp:63:7:63:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:63:7:63:7 | i | i | test.cpp:62:3:62:7 | call to scanf | scanf |
+| test.cpp:75:7:75:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:75:7:75:7 | i | i | test.cpp:74:3:74:7 | call to scanf | scanf |
+| test.cpp:87:7:87:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:87:7:87:7 | i | i | test.cpp:86:3:86:8 | call to fscanf | fscanf |
+| test.cpp:94:7:94:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:94:7:94:7 | i | i | test.cpp:93:3:93:8 | call to sscanf | sscanf |
+| test.cpp:143:8:143:8 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:143:8:143:8 | i | i | test.cpp:141:7:141:11 | call to scanf | scanf |
+| test.cpp:152:8:152:8 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:152:8:152:8 | i | i | test.cpp:150:7:150:11 | call to scanf | scanf |
+| test.cpp:184:8:184:8 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:184:8:184:8 | i | i | test.cpp:183:7:183:11 | call to scanf | scanf |
+| test.cpp:203:8:203:8 | j | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 2. | test.cpp:203:8:203:8 | j | j | test.cpp:200:7:200:11 | call to scanf | scanf |
+| test.cpp:227:9:227:9 | d | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 2. | test.cpp:227:9:227:9 | d | d | test.cpp:225:25:225:29 | call to scanf | scanf |
+| test.cpp:231:9:231:9 | d | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 2. | test.cpp:231:9:231:9 | d | d | test.cpp:229:14:229:18 | call to scanf | scanf |
+| test.cpp:243:7:243:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:243:7:243:7 | i | i | test.cpp:242:3:242:7 | call to scanf | scanf |
+| test.cpp:251:7:251:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:251:7:251:7 | i | i | test.cpp:250:3:250:7 | call to scanf | scanf |
+| test.cpp:259:7:259:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:259:7:259:7 | i | i | test.cpp:258:3:258:7 | call to scanf | scanf |
+| test.cpp:271:7:271:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:271:7:271:7 | i | i | test.cpp:270:3:270:7 | call to scanf | scanf |
+| test.cpp:281:8:281:12 | ptr_i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:281:8:281:12 | ptr_i | ptr_i | test.cpp:280:3:280:7 | call to scanf | scanf |
+| test.cpp:289:7:289:7 | i | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:289:7:289:7 | i | i | test.cpp:288:3:288:7 | call to scanf | scanf |
+| test.cpp:383:25:383:25 | u | $@ is read here, but may not have been written. It should be guarded by a check that $@() returns at least 1. | test.cpp:383:25:383:25 | u | u | test.cpp:382:6:382:11 | call to sscanf | sscanf |
@@ -0,0 +1 @@
+Critical/MissingCheckScanf.ql