[CPP-435] A much-improved IR query, still some false negatives.

Author: zlaski-semmle

cpp/ql/src/Likely Bugs/Memory Management/MemsetMayBeDeleted.ql

@@ -22,12 +22,20 @@ class MemsetCallInstruction extends CallInstruction {
 
 Instruction getAUseInstruction(Instruction insn) { result = insn.getAUse().getUse() }
 
+predicate pointsIntoStack(Instruction instr) {
+  instr.(VariableAddressInstruction).getIRVariable() instanceof IRAutomaticVariable
+  or
+  pointsIntoStack(instr.(CopyInstruction).getSourceValue())
+  or
+  pointsIntoStack(instr.(ConvertInstruction).getUnary())
+}
+
 from MemsetCallInstruction memset, SizedBufferMustWriteSideEffectInstruction sei
 where
   sei.getPrimaryInstruction() = memset and
-  forall(Instruction use | use = getAUseInstruction+(sei) | use instanceof ChiInstruction) and
-  exists(Instruction def | memset.getPositionalArgument(0) = getAUseInstruction+(def) |
-    def instanceof UninitializedInstruction
-  )
+  // The first argument to memset must reside on the stack
+  pointsIntoStack(valueNumber(memset.getPositionalArgument(0)).getAnInstruction()) and
+  // The result of memset may not be subsequently used
+  forall(Instruction use | use = getAUseInstruction+(sei) | use instanceof ChiInstruction)
 select memset,
   "Call to " + memset.getStaticCallTarget().getName() + " may be deleted by the compiler."

cpp/ql/test/query-tests/Likely Bugs/Memory Management/MemsetMayBeDeleted/MemsetMayBeDeleted.c

@@ -85,7 +85,7 @@ int func3(void) {
 int func4(void) { 
 	char pw1a[PW_SIZE];
 	use_pw(pw1a);
-	__builtin_memset(pw1a + 3, 0, PW_SIZE - 3); // BAD 
+	__builtin_memset(pw1a + 3, 0, PW_SIZE - 3); // BAD [NOT DETECTED]
 	return 0;
 }
 
@@ -115,7 +115,7 @@ int func5(void) {
 int func7(void) { 
 	char pw1a[PW_SIZE];
 	use_pw(pw1a);
-	__builtin_memset(&pw1a[3], 0, PW_SIZE - 5); // BAD
+	__builtin_memset(&pw1a[3], 0, PW_SIZE - 5); // BAD [NOT DETECTED]
 	return 0;
 }
 

cpp/ql/test/query-tests/Likely Bugs/Memory Management/MemsetMayBeDeleted/MemsetMayBeDeleted.cpp

@@ -48,7 +48,7 @@ void func3(unsigned long long sz) {
 // x64 msvc v19.22: deleted
 void func4(unsigned long long sz) {
     char buff[128]; 
-    memset(buff, 0, PW_SIZE); // BAD
+    memset(buff, 0, PW_SIZE); // BAD [NOT DETECTED]
     strcpy(buff, "Hello");
 }
 

cpp/ql/test/query-tests/Likely Bugs/Memory Management/MemsetMayBeDeleted/MemsetMayBeDeleted.expected

@@ -1,12 +1,8 @@
-WARNING: Unused predicate insnDominates (/mnt/c/code/ql/cpp/ql/src/Likely Bugs/Memory Management/MemsetMayBeDeleted.ql:27,19-32)
 | MemsetMayBeDeleted.c:19:2:19:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
 | MemsetMayBeDeleted.c:29:2:29:17 | Call: call to __builtin_memset | Call to __builtin_memset may be deleted by the compiler. |
 | MemsetMayBeDeleted.c:39:2:39:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
-| MemsetMayBeDeleted.c:59:2:59:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
-| MemsetMayBeDeleted.c:79:2:79:17 | Call: call to __builtin_memset | Call to __builtin_memset may be deleted by the compiler. |
-| MemsetMayBeDeleted.c:109:2:109:17 | Call: call to __builtin_memset | Call to __builtin_memset may be deleted by the compiler. |
-| MemsetMayBeDeleted.c:129:2:129:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
+| MemsetMayBeDeleted.c:68:2:68:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
+| MemsetMayBeDeleted.c:138:2:138:7 | Call: call to memset | Call to memset may be deleted by the compiler. |
 | MemsetMayBeDeleted.cpp:43:5:43:10 | Call: call to memset | Call to memset may be deleted by the compiler. |
-| MemsetMayBeDeleted.cpp:51:5:51:10 | Call: call to memset | Call to memset may be deleted by the compiler. |
 | MemsetMayBeDeleted.cpp:71:5:71:10 | Call: call to memset | Call to memset may be deleted by the compiler. |
 | MemsetMayBeDeleted.cpp:79:5:79:10 | Call: call to memset | Call to memset may be deleted by the compiler. |