Merge branch 'main' into github-only

Author: oscarsj

.github/workflows/qhelp-pr-preview.yml

@@ -77,7 +77,7 @@ jobs:
           done < "${RUNNER_TEMP}/paths.txt" >> comment_body.txt
           exit "${EXIT_CODE}"
 
-      - if: always()
+      - if: ${{ !cancelled() }}
         uses: actions/upload-artifact@v3
         with:
           name: comment

cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md

@@ -2,11 +2,8 @@ private import codeql.ssa.Ssa as SsaImplCommon
 private import semmle.code.cpp.ir.IR
 private import DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
-private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
 private import semmle.code.cpp.models.interfaces.Allocation as Alloc
 private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
-private import semmle.code.cpp.models.interfaces.FlowOutBarrier as FOB
-private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO
 private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import DataFlowPrivate
 private import ssa0.SsaInternals as SsaInternals0
@@ -797,30 +794,10 @@ private Node getAPriorDefinition(SsaDefOrUse defOrUse) {
   )
 }
 
-/**
- * Holds if there should not be use-use flow out of `n` (or a conversion that
- * flows to `n`).
- */
-private predicate modeledFlowBarrier(Node n) {
-  exists(FIO::FunctionInput input, CallInstruction call |
-    call.getStaticCallTarget().(FOB::FlowOutBarrierFunction).isFlowOutBarrier(input) and
-    n = callInput(call, input)
-  )
-  or
-  exists(Operand operand, Instruction instr, Node n0, int indirectionIndex |
-    modeledFlowBarrier(n0) and
-    nodeHasInstruction(n0, instr, indirectionIndex) and
-    conversionFlow(operand, instr, false, _) and
-    nodeHasOperand(n, operand, indirectionIndex)
-  )
-}
-
 /** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */
 predicate ssaFlow(Node nodeFrom, Node nodeTo) {
   exists(Node nFrom, boolean uncertain, SsaDefOrUse defOrUse |
-    ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and
-    not modeledFlowBarrier(nFrom) and
-    nodeFrom != nodeTo
+    ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and nodeFrom != nodeTo
   |
     if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(defOrUse)] else nodeFrom = nFrom
   )

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.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/lib/semmle/code/cpp/models/implementations/Memset.qll

@@ -1,15 +1,14 @@
 import semmle.code.cpp.models.interfaces.DataFlow
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
-import semmle.code.cpp.models.interfaces.FlowOutBarrier
 
 /**
  * The standard function `swap`. A use of `swap` looks like this:
  * ```
  * std::swap(obj1, obj2)
  * ```
  */
-private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
+private class Swap extends DataFlowFunction {
   Swap() { this.hasQualifiedName(["std", "bsl"], "swap") }
 
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
@@ -19,8 +18,6 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
     input.isParameterDeref(1) and
     output.isParameterDeref(0)
   }
-
-  override predicate isFlowOutBarrier(FunctionInput input) { input.isParameterDeref([0, 1]) }
 }
 
 /**
@@ -29,9 +26,7 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction {
  * obj1.swap(obj2)
  * ```
  */
-private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction,
-  FlowOutBarrierFunction
-{
+private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction {
   MemberSwap() {
     this.hasName("swap") and
     this.getNumberOfParameters() = 1 and
@@ -52,8 +47,4 @@ private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction,
   override predicate parameterEscapesOnlyViaReturn(int index) { index = 0 }
 
   override predicate parameterIsAlwaysReturned(int index) { index = 0 }
-
-  override predicate isFlowOutBarrier(FunctionInput input) {
-    input.isQualifierObject() or input.isParameterDeref(0)
-  }
 }

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

@@ -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/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll

@@ -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/dataflow-consistency.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/dataflow-ir-consistency.expected

@@ -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
 }

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

@@ -71,11 +71,11 @@ void test_pair()
 	sink(i.second); // $ MISSING: ast,ir
 	sink(i); // $ ast,ir
 	sink(j.first);
-	sink(j.second); // $ SPURIOUS: ast
-	sink(j); // $ SPURIOUS: ast
+	sink(j.second); // $ SPURIOUS: ast,ir
+	sink(j); // $ SPURIOUS: ast,ir
 	sink(k.first);
-	sink(k.second); // $ SPURIOUS: ast
-	sink(k); // $ SPURIOUS: ast
+	sink(k.second); // $ SPURIOUS: ast,ir
+	sink(k); // $ SPURIOUS: ast,ir
 	sink(l.first);
 	sink(l.second); // $ MISSING: ast,ir
 	sink(l); // $ ast,ir
@@ -196,10 +196,10 @@ void test_map()
 	sink(m18); // $ ast,ir
 	m15.swap(m16);
 	m17.swap(m18);
-	sink(m15); // $ SPURIOUS: ast
+	sink(m15); // $ SPURIOUS: ast,ir
 	sink(m16); // $ ast,ir
 	sink(m17); // $ ast,ir
-	sink(m18); // $ SPURIOUS: ast
+	sink(m18); // $ SPURIOUS: ast,ir
 
 	// merge
 	std::map<char *, char *> m19, m20, m21, m22;
@@ -345,10 +345,10 @@ void test_unordered_map()
 	sink(m18); // $ ast,ir
 	m15.swap(m16);
 	m17.swap(m18);
-	sink(m15); // $ SPURIOUS: ast
+	sink(m15); // $ SPURIOUS: ast,ir
 	sink(m16); // $ ast,ir
 	sink(m17); // $ ast,ir
-	sink(m18); // $ SPURIOUS: ast
+	sink(m18); // $ SPURIOUS: ast,ir
 
 	// merge
 	std::unordered_map<char *, char *> m19, m20, m21, m22;

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

@@ -81,10 +81,10 @@ void test_set()
 	sink(s15); // $ ast,ir
 	s12.swap(s13);
 	s14.swap(s15);
-	sink(s12); // $ SPURIOUS: ast
+	sink(s12); // $ SPURIOUS: ast,ir
 	sink(s13); // $ ast,ir
 	sink(s14); // $ ast,ir
-	sink(s15); // $ SPURIOUS: ast
+	sink(s15); // $ SPURIOUS: ast,ir
 
 	// merge
 	std::set<char *> s16, s17, s18, s19;
@@ -193,10 +193,10 @@ void test_unordered_set()
 	sink(s15); // $ ast,ir
 	s12.swap(s13);
 	s14.swap(s15);
-	sink(s12); // $ SPURIOUS: ast
+	sink(s12); // $ SPURIOUS: ast,ir
 	sink(s13); // $ ast,ir
 	sink(s14); // $ ast,ir
-	sink(s15); // $ SPURIOUS: ast
+	sink(s15); // $ SPURIOUS: ast,ir
 
 	// merge
 	std::unordered_set<char *> s16, s17, s18, s19;

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

@@ -280,9 +280,9 @@ void test_string_swap() {
 	s4.swap(s3);
 
 	sink(s1); // $ ast,ir
-	sink(s2); // $ SPURIOUS: ast
+	sink(s2); // $ SPURIOUS: ast,ir
 	sink(s3); // $ ast,ir
-	sink(s4); // $ SPURIOUS: ast
+	sink(s4); // $ SPURIOUS: ast,ir
 }
 
 void test_string_clear() {

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

@@ -118,9 +118,9 @@ void test_stringstream_swap()
 	ss4.swap(ss3);
 
 	sink(ss1); // $ ast,ir
-	sink(ss2); // $ SPURIOUS: ast
+	sink(ss2); // $ SPURIOUS: ast,ir
 	sink(ss3); // $ ast,ir
-	sink(ss4); // $ SPURIOUS: ast
+	sink(ss4); // $ SPURIOUS: ast,ir
 }
 
 void test_stringstream_in()

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

@@ -212,7 +212,7 @@ void test_swap() {
 
 	std::swap(x, y);
 
-	sink(x); // $ SPURIOUS: ast
+	sink(x); // $ SPURIOUS: ast,ir
 	sink(y); // $ ast,ir
 }
 

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

@@ -114,10 +114,10 @@ void test_vector_swap() {
 	v1.swap(v2);
 	v3.swap(v4);
 
-	sink(v1); // $ SPURIOUS: ast
+	sink(v1); // $ SPURIOUS: ast,ir
 	sink(v2); // $ ast,ir
 	sink(v3); // $ ast,ir
-	sink(v4); // $ SPURIOUS: ast
+	sink(v4); // $ SPURIOUS: ast,ir
 }
 
 void test_vector_clear() {