Update MEM30-C to include all pointer accesses

Nikita Kraiouchkine · Nikita Kraiouchkine · commit 90fb93b84b36 · 2023-02-02T15:29:05.000+01:00
diff --git a/c/cert/src/rules/MEM30-C/DoNotAccessFreedMemory.ql b/c/cert/src/rules/MEM30-C/DoNotAccessFreedMemory.ql
@@ -21,9 +21,10 @@ predicate isFreeExpr(Expr e, StackVariable v) {
   exists(VariableAccess va | va.getTarget() = v and freeExprOrIndirect(e, va, _))
 }
 
-/** `e` is an expression that (may) dereference `v`. */
-predicate isDerefExpr(Expr e, StackVariable v) {
-  v.getAnAccess() = e and dereferenced(e)
+/** `e` is an expression that accesses `v` but is not the lvalue of an assignment. */
+predicate isAccessExpr(Expr e, StackVariable v) {
+  v.getAnAccess() = e and
+  not exists(Assignment a | a.getLValue() = e)
   or
   isDerefByCallExpr(_, _, e, v)
 }
@@ -38,26 +39,28 @@ predicate isDerefByCallExpr(Call c, int i, VariableAccess va, StackVariable v) {
   v.getAnAccess() = va and
   va = c.getAnArgumentSubExpr(i) and
   not c.passesByReference(i, va) and
-  (c.getTarget().hasEntryPoint() implies isDerefExpr(_, c.getTarget().getParameter(i)))
+  (c.getTarget().hasEntryPoint() implies isAccessExpr(_, c.getTarget().getParameter(i)))
 }
 
 class UseAfterFreeReachability extends StackVariableReachability {
   UseAfterFreeReachability() { this = "UseAfterFree" }
 
   override predicate isSource(ControlFlowNode node, StackVariable v) { isFreeExpr(node, v) }
 
-  override predicate isSink(ControlFlowNode node, StackVariable v) { isDerefExpr(node, v) }
+  override predicate isSink(ControlFlowNode node, StackVariable v) { isAccessExpr(node, v) }
 
   override predicate isBarrier(ControlFlowNode node, StackVariable v) {
     definitionBarrier(v, node) or
     isFreeExpr(node, v)
   }
 }
 
+// This query is a modified version of the `UseAfterFree.ql`
+// (cpp/use-after-free) query from the CodeQL standard library.
 from UseAfterFreeReachability r, StackVariable v, Expr free, Expr e
 where
   not isExcluded(e, InvalidMemory1Package::doNotAccessFreedMemoryQuery()) and
   r.reaches(free, v, e)
 select e,
-  "Memory pointed to by '" + v.getName().toString() +
-    "' accessed but may have been previously freed $@.", free, "here"
+  "Pointer '" + v.getName().toString() + "' accessed but may have been previously freed $@.", free,
+  "here"
diff --git a/c/cert/test/rules/MEM30-C/DoNotAccessFreedMemory.expected b/c/cert/test/rules/MEM30-C/DoNotAccessFreedMemory.expected
@@ -1,2 +1,5 @@
-| test.c:11:47:11:47 | p | Memory pointed to by 'p' accessed but may have been previously freed $@. | test.c:12:5:12:8 | call to free | here |
-| test.c:25:10:25:12 | buf | Memory pointed to by 'buf' accessed but may have been previously freed $@. | test.c:24:3:24:6 | call to free | here |
+| test.c:11:47:11:47 | p | Pointer 'p' accessed but may have been previously freed $@. | test.c:12:5:12:8 | call to free | here |
+| test.c:25:10:25:12 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:24:3:24:6 | call to free | here |
+| test.c:32:15:32:17 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |
+| test.c:33:9:33:11 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |
+| test.c:34:16:34:18 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |
diff --git a/c/cert/test/rules/MEM30-C/test.c b/c/cert/test/rules/MEM30-C/test.c
@@ -23,4 +23,16 @@ void test_freed_arg(char *input) {
   strcpy(buf, input); // COMPLIANT
   free(buf);
   strcpy(buf, input); // NON_COMPLIANT
+}
+
+void test_freed_access_no_deref(char *input) {
+  char *buf = (char *)malloc(strlen(input) + 1);
+  strcpy(buf, input); // COMPLIANT
+  free(buf);
+  char *tmp = buf;  // NON_COMPLIANT
+  tmp = buf + 1;    // NON_COMPLIANT
+  char *tmp2 = buf; // NON_COMPLIANT
+  buf = NULL;       // COMPLIANT
+  (char *)buf;      // COMPLIANT
+  tmp2 = buf + 1;   // COMPLIANT
 }
