Compiler optimization will exclude the cleaning of private information.
+Using the memset function to clear private data in a variable that has no subsequent use is potentially dangerous, since the compiler can remove the call.
+For some compilers, optimization is also possible when using calls to free memory after the memset function.
It is possible to miss detection of vulnerabilities if used to clear fields of structures or parts of a buffer.
+ +We recommend to use the RtlSecureZeroMemory or memset_s functions, or compilation flags that exclude optimization of memset calls (e.g. -fno-builtin-memset).
The following example demonstrates an erroneous and corrected use of the memset function.
memset the function to clear private data in a variable that has no subsequent use
+ * is potentially dangerous because the compiler can remove the call.
+ * @kind problem
+ * @id cpp/compiler-removal-of-code-to-clear-buffers
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ * external/cwe/cwe-14
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.dataflow.StackAddress
+
+/**
+ * A call to `memset` of the form `memset(ptr, value, num)`, for some local variable `ptr`.
+ */
+class CompilerRemovaMemset extends FunctionCall {
+ CompilerRemovaMemset() {
+ this.getTarget().hasGlobalOrStdName("memset") and
+ exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
+ DataFlow::localFlow(source, sink) and
+ this.getArgument(0) = isv.getAnAccess() and
+ (
+ source.asExpr() = exp
+ or
+ // handle the case where exp is defined by an address being passed into some function.
+ source.asDefiningArgument() = exp
+ ) and
+ exp.getLocation().getEndLine() < this.getArgument(0).getLocation().getStartLine() and
+ sink.asExpr() = this.getArgument(0)
+ )
+ }
+
+ predicate isExistsAllocForThisVariable() {
+ exists(AllocationExpr alloc, Variable v |
+ alloc = v.getAnAssignedValue() and
+ this.getArgument(0) = v.getAnAccess() and
+ alloc.getASuccessor+() = this
+ )
+ or
+ not stackPointerFlowsToUse(this.getArgument(0), _, _, _)
+ }
+
+ predicate isExistsFreeForThisVariable() {
+ exists(DeallocationExpr free, Variable v |
+ this.getArgument(0) = v.getAnAccess() and
+ free.getFreedExpr() = v.getAnAccess() and
+ this.getASuccessor+() = free
+ )
+ }
+
+ predicate isExistsCallWithThisVariableExcludingDeallocationCalls() {
+ exists(FunctionCall fc, Variable v |
+ not fc instanceof DeallocationExpr and
+ this.getArgument(0) = v.getAnAccess() and
+ fc.getAnArgument() = v.getAnAccess() and
+ this.getASuccessor+() = fc
+ )
+ }
+
+ predicate isVariableUseAfterMemsetExcludingCalls() {
+ exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
+ DataFlow::localFlow(source, sink) and
+ this.getArgument(0) = isv.getAnAccess() and
+ source.asExpr() = isv.getAnAccess() and
+ exp.getLocation().getStartLine() > this.getArgument(2).getLocation().getEndLine() and
+ not exp.getParent() instanceof FunctionCall and
+ sink.asExpr() = exp
+ )
+ }
+
+ predicate isVariableUseBoundWithArgumentFunction() {
+ exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Parameter p, Expr exp |
+ DataFlow::localFlow(source, sink) and
+ this.getArgument(0) = isv.getAnAccess() and
+ this.getEnclosingFunction().getAParameter() = p and
+ exp.getAChild*() = p.getAnAccess() and
+ source.asExpr() = exp and
+ sink.asExpr() = isv.getAnAccess()
+ )
+ }
+
+ predicate isVariableUseBoundWithGlobalVariable() {
+ exists(
+ DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, GlobalVariable gv, Expr exp
+ |
+ DataFlow::localFlow(source, sink) and
+ this.getArgument(0) = isv.getAnAccess() and
+ exp.getAChild*() = gv.getAnAccess() and
+ source.asExpr() = exp and
+ sink.asExpr() = isv.getAnAccess()
+ )
+ }
+
+ predicate isExistsCompilationFlagsBlockingRemoval() {
+ exists(Compilation c |
+ c.getAFileCompiled() = this.getFile() and
+ c.getAnArgument() = "-fno-builtin-memset"
+ )
+ }
+
+ predicate isUseVCCompilation() {
+ exists(Compilation c |
+ c.getAFileCompiled() = this.getFile() and
+ (
+ c.getArgument(2).matches("%gcc%") or
+ c.getArgument(2).matches("%g++%") or
+ c.getArgument(2).matches("%clang%") or
+ c.getArgument(2) = "--force-recompute"
+ )
+ )
+ }
+}
+
+from CompilerRemovaMemset fc
+where
+ not (fc.isExistsAllocForThisVariable() and not fc.isExistsFreeForThisVariable()) and
+ not (fc.isExistsFreeForThisVariable() and not fc.isUseVCCompilation()) and
+ not fc.isVariableUseAfterMemsetExcludingCalls() and
+ not fc.isExistsCallWithThisVariableExcludingDeallocationCalls() and
+ not fc.isVariableUseBoundWithArgumentFunction() and
+ not fc.isVariableUseBoundWithGlobalVariable() and
+ not fc.isExistsCompilationFlagsBlockingRemoval()
+select fc.getArgument(0), "This variable will not be cleared."
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.expected
new file mode 100644
index 000000000000..8d98d94ff467
--- /dev/null
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.expected
@@ -0,0 +1,3 @@
+| test.c:13:9:13:13 | buff1 | This variable will not be cleared. |
+| test.c:35:9:35:13 | buff1 | This variable will not be cleared. |
+| test.c:43:9:43:13 | buff1 | This variable will not be cleared. |
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.qlref
new file mode 100644
index 000000000000..61d2a29b1264
--- /dev/null
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/CompilerRemovalOfCodeToClearBuffers.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.ql
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/test.c b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/test.c
new file mode 100644
index 000000000000..221072330c3f
--- /dev/null
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-14/semmle/tests/test.c
@@ -0,0 +1,201 @@
+struct buffers
+{
+ unsigned char buff1[50];
+ unsigned char *buff2;
+} globalBuff1,*globalBuff2;
+
+unsigned char * globalBuff;
+void badFunc0_0(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+}
+void nobadFunc0_0(){
+ unsigned char buff1[12];
+ memset(buff1,12,12);
+}
+void nobadFunc0_1(){
+ unsigned char buff1[12];
+ int i;
+ memset(buff1,12,12);
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ free(buff1);
+}
+void nobadFunc1_0(){
+ unsigned char * buff1;
+ buff1 = (unsigned char *) malloc(12);
+ memset(buff1,12,12);
+}
+void badFunc1_0(){
+ unsigned char * buff1;
+ buff1 = (unsigned char *) malloc(12);
+ memset(buff1,12,12);
+ free(buff1);
+}
+void badFunc1_1(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ free(buff1);
+}
+void nobadFunc2_0_0(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ printf(buff1);
+}
+
+void nobadFunc2_0_1(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ printf(buff1+3);
+}
+
+void nobadFunc2_0_2(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ printf(*buff1);
+}
+
+void nobadFunc2_0_3(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ printf(*(buff1+3));
+}
+unsigned char * nobadFunc2_0_4(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ return buff1;
+}
+
+unsigned char * nobadFunc2_0_5(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ return buff1+3;
+}
+unsigned char nobadFunc2_0_6(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ return *buff1;
+}
+
+unsigned char nobadFunc2_0_7(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ return *(buff1+3);
+}
+void nobadFunc2_1_0(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ if(*buff1==0)
+ printf("123123");
+}
+void nobadFunc2_1_1(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ if(*(buff1+3)==0)
+ printf("123123");
+}
+void nobadFunc2_1_2(){
+ unsigned char buff1[12];
+ int i;
+ for(i=0;i<12;i++)
+ buff1[i]=13;
+ memset(buff1,12,12);
+ buff1[2]=5;
+}
+void nobadFunc3_0(unsigned char * buffAll){
+ unsigned char * buff1 = buffAll;
+ memset(buff1,12,12);
+}
+void nobadFunc3_1(unsigned char * buffAll){
+ unsigned char * buff1 = buffAll+3;
+ memset(buff1,12,12);
+}
+void nobadFunc3_2(struct buffers buffAll){
+ unsigned char * buff1 = buffAll.buff1;
+ memset(buff1,12,12);
+}
+void nobadFunc3_3(struct buffers buffAll){
+ unsigned char * buff1 = buffAll.buff2;
+ memset(buff1,12,12);
+}
+void nobadFunc3_4(struct buffers buffAll){
+ unsigned char * buff1 = buffAll.buff2+3;
+ memset(buff1,12,12);
+}
+void nobadFunc3_5(struct buffers * buffAll){
+ unsigned char * buff1 = buffAll->buff1;
+ memset(buff1,12,12);
+}
+void nobadFunc3_6(struct buffers *buffAll){
+ unsigned char * buff1 = buffAll->buff2;
+ memset(buff1,12,12);
+}
+void nobadFunc4(){
+ unsigned char * buff1 = globalBuff;
+ memset(buff1,12,12);
+}
+void nobadFunc4_0(){
+ unsigned char * buff1 = globalBuff;
+ memset(buff1,12,12);
+}
+void nobadFunc4_1(){
+ unsigned char * buff1 = globalBuff+3;
+ memset(buff1,12,12);
+}
+void nobadFunc4_2(){
+ unsigned char * buff1 = globalBuff1.buff1;
+ memset(buff1,12,12);
+}
+void nobadFunc4_3(){
+ unsigned char * buff1 = globalBuff1.buff2;
+ memset(buff1,12,12);
+}
+void nobadFunc4_4(){
+ unsigned char * buff1 = globalBuff1.buff2+3;
+ memset(buff1,12,12);
+}
+void nobadFunc4_5(){
+ unsigned char * buff1 = globalBuff2->buff1;
+ memset(buff1,12,12);
+}
+void nobadFunc4_6(){
+ unsigned char * buff1 = globalBuff2->buff2;
+ memset(buff1,12,12);
+}
+