C++: Insecure memset

cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll

@@ -1,11 +1,14 @@
+import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.ArrayFunction
 import semmle.code.cpp.models.interfaces.DataFlow
+import semmle.code.cpp.models.interfaces.SideEffect
 import semmle.code.cpp.models.interfaces.Taint
 
 /**
  * The standard function `strcpy` and its wide, sized, and Microsoft variants.
  */
-class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
+class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, AliasFunction,
+  SideEffectFunction {
   StrcpyFunction() {
     this.hasName("strcpy") or
     this.hasName("_mbscpy") or
@@ -86,4 +89,22 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction {
       output.isReturnValueDeref()
     )
   }
+
+  override predicate hasOnlySpecificReadSideEffects() { any() }
+
+  override predicate hasOnlySpecificWriteSideEffects() { any() }
+
+  override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
+    i = 1 and buffer = true
+  }
+
+  override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
+    i = 0 and buffer = true and mustWrite = true
+  }
+
+  override predicate parameterNeverEscapes(int index) { index = 1 }
+
+  override predicate parameterEscapesOnlyViaReturn(int index) { index = 0 }
+
+  override predicate parameterIsAlwaysReturned(int index) { index = 0 }
 }

cpp/ql/test/library-tests/ir/memset/MemsetMayBeDeleted.c

@@ -0,0 +1,140 @@
+typedef unsigned long long size_t;
+void *memset(void *s, int c, unsigned long n);
+void *__builtin_memset(void *s, int c, unsigned long n);
+typedef int errno_t;
+typedef unsigned int rsize_t;
+errno_t memset_s(void *dest, rsize_t destsz, int ch, rsize_t count);
+char *strcpy(char *dest, const char *src);
+
+extern void use_pw(char *pw);
+
+#define PW_SIZE 32
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): deleted
+int func1(void) { 
+	char pw1[PW_SIZE];
+	use_pw(pw1);
+	memset(pw1, 0, PW_SIZE); // BAD
+	return 0;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func1a(void) {  
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(pw1a, 0, PW_SIZE); // BAD
+	return 0;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): deleted
+char *func1b(void) {  
+	char pw1b[PW_SIZE];
+	use_pw(pw1b);
+	memset(pw1b, 0, PW_SIZE); // BAD
+	pw1b[0] = pw1b[3] = 'a';
+	return 0;
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func1c(char pw1c[PW_SIZE]) {  
+	use_pw(pw1c);
+	memset(pw1c, 0, PW_SIZE); // GOOD
+	return 0;
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.14 (WINE): not deleted
+char pw1d[PW_SIZE];
+int func1d() {  
+	use_pw(pw1d);
+	memset(pw1d, 0, PW_SIZE); // GOOD
+	return 0;
+}
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.14 (WINE): not deleted
+char *func2(void) {  
+	char pw2[PW_SIZE];
+	use_pw(pw2);
+	memset(pw2, 1, PW_SIZE); // BAD
+	return pw2;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): partially deleted
+int func3(void) { 
+	char pw3[PW_SIZE];
+	use_pw(pw3);
+	memset(pw3, 4, PW_SIZE); // BAD [NOT DETECTED]
+	return pw3[2];
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func4(void) { 
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(pw1a + 3, 0, PW_SIZE - 3); // BAD [NOT DETECTED]
+	return 0;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func6(void) { 
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(&pw1a[3], 0, PW_SIZE - 3); // BAD [NOT DETECTED]
+	return pw1a[2];
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func5(void) { 
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(pw1a + 3, 0, PW_SIZE - 4); // GOOD
+	return pw1a[4];
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func7(void) { 
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(&pw1a[3], 0, PW_SIZE - 5); // BAD [NOT DETECTED]
+	return 0;
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.14 (WINE): not deleted
+int func8(void) { 
+	char pw1a[PW_SIZE];
+	use_pw(pw1a);
+	__builtin_memset(pw1a + pw1a[3], 0, PW_SIZE - 4); // GOOD
+	return pw1a[4];
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.14 (WINE): not deleted
+char *func9(void) {
+	char pw1[PW_SIZE];
+	use_pw(pw1);
+	memset(pw1, 0, PW_SIZE); // BAD
+	return 0;
+}

cpp/ql/test/library-tests/ir/memset/MemsetMayBeDeleted.cpp

@@ -0,0 +1,184 @@
+extern "C" {
+	typedef unsigned long long size_t;
+	void *memset(void *s, int c, unsigned long n);
+	void *__builtin_memset(void *s, int c, unsigned long n);
+	typedef int errno_t;
+	typedef unsigned int rsize_t;
+	errno_t memset_s(void *dest, rsize_t destsz, int ch, rsize_t count);
+	char *strcpy(char *dest, const char *src);
+	void *memcpy(void *dest, const void *src, unsigned long n);
+	void *malloc(unsigned long size);
+	void free(void *ptr);
+	extern void use_pw(char *pw);
+}
+
+#define PW_SIZE 32
+
+struct mem {
+	int a;
+	char b[PW_SIZE];
+	int c;
+};
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.22: not deleted
+void func(char buff[128], unsigned long long sz) {
+    memset(buff, 0, PW_SIZE); // GOOD
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.22: not deleted
+char *func2(char buff[128], unsigned long long sz) { 
+    memset(buff, 0, PW_SIZE); // GOOD
+    return buff;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: deleted
+void func3(unsigned long long sz) { 
+    char buff[128]; 
+    memset(buff, 0, PW_SIZE); // BAD
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: deleted
+void func4(unsigned long long sz) {
+    char buff[128]; 
+    memset(buff, 0, PW_SIZE); // BAD [NOT DETECTED]
+    strcpy(buff, "Hello");
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: deleted
+void func5(unsigned long long sz) {
+    char buff[128]; 
+    memset(buff, 0, PW_SIZE); // BAD [NOT DETECTED]
+    if (sz > 5) {
+        strcpy(buff, "Hello");
+    }
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: deleted
+void func6(unsigned long long sz) {
+    struct mem m; 
+    memset(&m, 0, PW_SIZE); // BAD
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: deleted
+void func7(unsigned long long sz) { 
+    struct mem m; 
+    memset(&m, 0, PW_SIZE); // BAD
+    m.a = 15;
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: not deleted
+void func8(unsigned long long sz) {
+    struct mem *m = (struct mem *)malloc(sizeof(struct mem));
+    memset(m, 0, PW_SIZE); // BAD [NOT DETECTED]
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: not deleted
+void func9(unsigned long long sz) {
+    struct mem *m = (struct mem *)malloc(sizeof(struct mem));
+    memset(m, 0, PW_SIZE); // BAD [NOT DETECTED]
+    free(m);
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: not deleted
+void func10(unsigned long long sz) {
+    struct mem *m = (struct mem *)malloc(sizeof(struct mem));
+    memset(m, 0, PW_SIZE); // BAD [NOT DETECTED]
+    m->a = sz;
+    m->c = m->a + 1; 
+}
+
+// x86-64 gcc 9.2: deleted
+// x86-64 clang 9.0.0: deleted
+// x64 msvc v19.22: not deleted
+void func11(unsigned long long sz) {
+    struct mem *m = (struct mem *)malloc(sizeof(struct mem));
+    ::memset(m, 0, PW_SIZE); // BAD [NOT DETECTED]
+    if (sz > 5) {
+    	strcpy(m->b, "Hello");
+    }
+}
+
+// x86-64 gcc 9.2: not deleted
+// x86-64 clang 9.0.0: not deleted
+// x64 msvc v19.22: not deleted
+int func12(unsigned long long sz) {
+    struct mem *m = (struct mem *)malloc(sizeof(struct mem));
+    memset(m, 0, sz); // GOOD
+    return m->c;
+}
+
+int funcN1() {
+	char pw[PW_SIZE];
+	char *pw_ptr = pw;
+	memset(pw, 0, PW_SIZE); // GOOD
+	use_pw(pw_ptr);
+	return 0;
+}
+
+char pw_global[PW_SIZE];
+int funcN2() {
+	use_pw(pw_global);
+	memset(pw_global, 0, PW_SIZE); // GOOD
+	return 0;
+}
+
+int funcN3(unsigned long long sz) {
+    struct mem m;
+    memset(&m, 0, sizeof(m)); // GOOD
+    return m.a;
+}
+
+void funcN(int num) {
+	char pw[PW_SIZE];
+	int i;
+
+	for (i = 0; i < num; i++)
+	{
+		use_pw(pw);
+		memset(pw, 0, PW_SIZE); // GOOD
+	}
+}
+
+class MyClass
+{
+public:
+	void set(int _x) {
+		x = _x;
+	}
+
+	int get()
+	{
+		return x;
+	}
+
+	void clear1() {
+		memset(&x, 0, sizeof(x)); // GOOD
+	}
+
+	void clear2() {
+		memset(&(this->x), 0, sizeof(this->x)); // GOOD
+	}
+
+private:
+	int x;
+};

cpp/ql/test/library-tests/ir/memset/MemsetMayBeDeleted.ql

@@ -0,0 +1,54 @@
+/**
+ * @name Call to `memset` may be deleted
+ * @description Calling `memset` or `bzero` on a buffer in order to clear its contents may get optimized away
+ *              by the compiler if said buffer is not subsequently used.  This is not desirable
+ *              behavior if the buffer contains sensitive data that could be exploited by an attacker.
+ * @kind problem
+ * @problem.severity recommendation
+ * @precision high
+ * @id cpp/memset-may-be-deleted
+ * @tags security
+ *       external/cwe/cwe-14
+ */
+
+import cpp
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.ValueNumbering
+import semmle.code.cpp.models.implementations.Memset
+
+class MemsetCallInstruction extends CallInstruction {
+  MemsetCallInstruction() { this.getStaticCallTarget() instanceof MemsetFunction }
+}
+
+private predicate isAlwaysLive(Instruction instr) {
+  instr instanceof ReturnValueInstruction or
+  instr instanceof ThrowValueInstruction or
+  instr instanceof AliasedUseInstruction
+}
+
+private predicate isLive(Instruction instr) {
+  isAlwaysLive(instr) or
+  isLive(instr.getAUse().getUse()) or
+  isLive(instr.(SideEffectInstruction).getPrimaryInstruction())
+}
+
+predicate pointsIntoStack(Instruction instr) {
+  instr.(VariableAddressInstruction).getIRVariable() instanceof IRAutomaticVariable
+  or
+  pointsIntoStack(instr.(CopyInstruction).getSourceValue())
+  or
+  pointsIntoStack(instr.(ConvertInstruction).getUnary())
+  or
+  pointsIntoStack(instr.(PointerArithmeticInstruction).getLeft())
+}
+
+from MemsetCallInstruction memset, SizedBufferMustWriteSideEffectInstruction sei
+where
+  sei.getPrimaryInstruction() = memset and
+  // 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)
+  not isLive(sei)
+select memset,
+  "Call to " + memset.getStaticCallTarget().getName() + " may be deleted by the compiler."