Merge pull request #15587 from MathiasVP/fix-memset-model

C++: Fix `memset` model

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll

@@ -22,11 +22,28 @@ private class MemsetFunctionModel extends ArrayFunction, DataFlowFunction, Alias
       ])
   }
 
+  /**
+   * Gets the index of the parameter that specifies the fill character to insert, if any.
+   */
+  private int getFillCharParameterIndex() {
+    (
+      this.hasGlobalOrStdOrBslName("memset")
+      or
+      this.hasGlobalOrStdName("wmemset")
+      or
+      this.hasGlobalName(["__builtin_memset", "__builtin_memset_chk"])
+    ) and
+    result = 1
+  }
+
   override predicate hasArrayOutput(int bufParam) { bufParam = 0 }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
     output.isReturnValue()
+    or
+    input.isParameter(this.getFillCharParameterIndex()) and
+    (output.isParameterDeref(0) or output.isReturnValueDeref())
   }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected

@@ -165,6 +165,7 @@ postWithInFlow
 | test.cpp:931:5:931:18 | global_pointer [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:932:5:932:19 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:932:6:932:19 | global_pointer [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:1045:9:1045:11 | ref arg buf | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
 uniqueParameterNodePosition

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected

@@ -25,6 +25,7 @@ postWithInFlow
 | test.cpp:391:10:391:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 | test.cpp:400:10:400:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 | test.cpp:407:10:407:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
+| test.cpp:1045:9:1045:11 | memset output argument | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
 uniqueParameterNodePosition

cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

@@ -309,6 +309,7 @@ irFlow
 | test.cpp:994:18:994:32 | *call to indirect_source | test.cpp:1003:19:1003:28 | *translated |
 | test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1027:19:1027:28 | *translated |
 | test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1031:19:1031:28 | *translated |
+| test.cpp:1045:14:1045:19 | call to source | test.cpp:1046:7:1046:10 | * ... |
 | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
 | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
 | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

@@ -1037,4 +1037,11 @@ namespace test_gettext {
     sink(translated); // clean 
     indirect_sink(translated); // clean
   }
+}
+
+void* memset(void*, int, size_t);
+
+void memset_test(char* buf) { // $ ast-def=buf
+	memset(buf, source(), 10);
+	sink(*buf); // $ ir MISSING: ast
 }