Merge branch 'github:main' into main

Author: mbaluda

.github/workflows/compile-queries.yml

@@ -46,7 +46,7 @@ jobs:
         with:
           channel: 'release'
       - name: check formatting
-        run: codeql query format */ql/**/*.{qll,ql} --check-only
+        run: find */ql -type f \( -name "*.qll" -o -name "*.ql" \) -print0 | xargs -0 codeql query format --check-only
       - name: compile queries - check-only
         # run with --check-only if running in a PR (github.sha != main)
         if : ${{ github.event_name == 'pull_request' }}

.github/workflows/swift.yml

@@ -111,4 +111,10 @@ jobs:
       - uses: actions/upload-artifact@v3
         with:
           name: swift-generated-cpp-files
-          path: swift/generated-cpp-files/**
+          path: swift/generated-cpp-files/**
+  database-upgrade-scripts:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: ./.github/actions/fetch-codeql
+      - uses: ./swift/actions/database-upgrade-scripts

config/identical-files.json

@@ -486,40 +486,6 @@
     "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll",
     "ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll"
   ],
-  "ReDoS Util Python/JS/Ruby/Java": [
-    "javascript/ql/lib/semmle/javascript/security/regexp/NfaUtils.qll",
-    "python/ql/lib/semmle/python/security/regexp/NfaUtils.qll",
-    "ruby/ql/lib/codeql/ruby/security/regexp/NfaUtils.qll",
-    "java/ql/lib/semmle/code/java/security/regexp/NfaUtils.qll"
-  ],
-  "ReDoS Exponential Python/JS/Ruby/Java": [
-    "javascript/ql/lib/semmle/javascript/security/regexp/ExponentialBackTracking.qll",
-    "python/ql/lib/semmle/python/security/regexp/ExponentialBackTracking.qll",
-    "ruby/ql/lib/codeql/ruby/security/regexp/ExponentialBackTracking.qll",
-    "java/ql/lib/semmle/code/java/security/regexp/ExponentialBackTracking.qll"
-  ],
-  "ReDoS Polynomial Python/JS/Ruby/Java": [
-    "javascript/ql/lib/semmle/javascript/security/regexp/SuperlinearBackTracking.qll",
-    "python/ql/lib/semmle/python/security/regexp/SuperlinearBackTracking.qll",
-    "ruby/ql/lib/codeql/ruby/security/regexp/SuperlinearBackTracking.qll",
-    "java/ql/lib/semmle/code/java/security/regexp/SuperlinearBackTracking.qll"
-  ],
-  "RegexpMatching Python/JS/Ruby": [
-    "javascript/ql/lib/semmle/javascript/security/regexp/RegexpMatching.qll",
-    "python/ql/lib/semmle/python/security/regexp/RegexpMatching.qll",
-    "ruby/ql/lib/codeql/ruby/security/regexp/RegexpMatching.qll"
-  ],
-  "BadTagFilterQuery Python/JS/Ruby": [
-    "javascript/ql/lib/semmle/javascript/security/BadTagFilterQuery.qll",
-    "python/ql/lib/semmle/python/security/BadTagFilterQuery.qll",
-    "ruby/ql/lib/codeql/ruby/security/BadTagFilterQuery.qll"
-  ],
-  "OverlyLargeRange Python/JS/Ruby/Java": [
-    "javascript/ql/lib/semmle/javascript/security/OverlyLargeRangeQuery.qll",
-    "python/ql/lib/semmle/python/security/OverlyLargeRangeQuery.qll",
-    "ruby/ql/lib/codeql/ruby/security/OverlyLargeRangeQuery.qll",
-    "java/ql/lib/semmle/code/java/security/OverlyLargeRangeQuery.qll"
-  ],
   "CFG": [
     "csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImplShared.qll",
     "ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImplShared.qll",

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -136,6 +136,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll

@@ -136,6 +136,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -136,6 +136,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {

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

@@ -16,21 +16,23 @@ private class StdBasicString extends ClassTemplateInstantiation {
 }
 
 /**
- * Additional model for `std::string` constructors that reference the character
- * type of the container, or an iterator.  For example construction from
- * iterators:
- * ```
- * std::string b(a.begin(), a.end());
- * ```
+ * The `std::basic_string::iterator` declaration.
  */
-private class StdStringConstructor extends Constructor, TaintFunction {
-  StdStringConstructor() { this.getDeclaringType() instanceof StdBasicString }
+private class StdBasicStringIterator extends Iterator, Type {
+  StdBasicStringIterator() {
+    this.getEnclosingElement() instanceof StdBasicString and this.hasName("iterator")
+  }
+}
 
+/**
+ * A `std::string` function for which taint should be propagated.
+ */
+abstract private class StdStringTaintFunction extends TaintFunction {
   /**
    * Gets the index of a parameter to this function that is a string (or
    * character).
    */
-  int getAStringParameterIndex() {
+  final int getAStringParameterIndex() {
     exists(Type paramType | paramType = this.getParameter(result).getUnspecifiedType() |
       // e.g. `std::basic_string::CharT *`
       paramType instanceof PointerType
@@ -41,15 +43,28 @@ private class StdStringConstructor extends Constructor, TaintFunction {
         this.getDeclaringType().getTemplateArgument(2).(Type).getUnspecifiedType()
       or
       // i.e. `std::basic_string::CharT`
-      this.getParameter(result).getUnspecifiedType() =
-        this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType()
+      paramType = this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType()
     )
   }
 
   /**
    * Gets the index of a parameter to this function that is an iterator.
    */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
+  final int getAnIteratorParameterIndex() {
+    this.getParameter(result).getType() instanceof Iterator
+  }
+}
+
+/**
+ * Additional model for `std::string` constructors that reference the character
+ * type of the container, or an iterator.  For example construction from
+ * iterators:
+ * ```
+ * std::string b(a.begin(), a.end());
+ * ```
+ */
+private class StdStringConstructor extends Constructor, StdStringTaintFunction {
+  StdStringConstructor() { this.getDeclaringType() instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from any parameter of the value type to the returned object
@@ -68,7 +83,7 @@ private class StdStringConstructor extends Constructor, TaintFunction {
 /**
  * The `std::string` function `c_str`.
  */
-private class StdStringCStr extends TaintFunction {
+private class StdStringCStr extends StdStringTaintFunction {
   StdStringCStr() { this.getClassAndName("c_str") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -81,7 +96,7 @@ private class StdStringCStr extends TaintFunction {
 /**
  * The `std::string` function `data`.
  */
-private class StdStringData extends TaintFunction {
+private class StdStringData extends StdStringTaintFunction {
   StdStringData() { this.getClassAndName("data") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -99,7 +114,7 @@ private class StdStringData extends TaintFunction {
 /**
  * The `std::string` function `push_back`.
  */
-private class StdStringPush extends TaintFunction {
+private class StdStringPush extends StdStringTaintFunction {
   StdStringPush() { this.getClassAndName("push_back") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -112,7 +127,7 @@ private class StdStringPush extends TaintFunction {
 /**
  * The `std::string` functions `front` and `back`.
  */
-private class StdStringFrontBack extends TaintFunction {
+private class StdStringFrontBack extends StdStringTaintFunction {
   StdStringFrontBack() { this.getClassAndName(["front", "back"]) instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -125,7 +140,7 @@ private class StdStringFrontBack extends TaintFunction {
 /**
  * The (non-member) `std::string` function `operator+`.
  */
-private class StdStringPlus extends TaintFunction {
+private class StdStringPlus extends StdStringTaintFunction {
   StdStringPlus() {
     this.hasQualifiedName(["std", "bsl"], "operator+") and
     this.getUnspecifiedType() instanceof StdBasicString
@@ -142,31 +157,15 @@ private class StdStringPlus extends TaintFunction {
 }
 
 /**
- * The `std::string` functions `operator+=`, `append`, `insert` and
- * `replace`. All of these functions combine the existing string
- * with a new string (or character) from one of the arguments.
+ * The `std::string` functions `operator+=`, `append` and `replace`.
+ * All of these functions combine the existing string with a new
+ * string (or character) from one of the arguments.
  */
-private class StdStringAppend extends TaintFunction {
+private class StdStringAppend extends StdStringTaintFunction {
   StdStringAppend() {
-    this.getClassAndName(["operator+=", "append", "insert", "replace"]) instanceof StdBasicString
-  }
-
-  /**
-   * Gets the index of a parameter to this function that is a string (or
-   * character).
-   */
-  int getAStringParameterIndex() {
-    this.getParameter(result).getType() instanceof PointerType or // e.g. `std::basic_string::CharT *`
-    this.getParameter(result).getType() instanceof ReferenceType or // e.g. `std::basic_string &`
-    this.getParameter(result).getUnspecifiedType() =
-      this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. `std::basic_string::CharT`
+    this.getClassAndName(["operator+=", "append", "replace"]) instanceof StdBasicString
   }
 
-  /**
-   * Gets the index of a parameter to this function that is an iterator.
-   */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
-
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from string and parameter to string (qualifier) and return value
     (
@@ -187,26 +186,42 @@ private class StdStringAppend extends TaintFunction {
 }
 
 /**
- * The standard function `std::string.assign`.
+ * The `std::string` function `insert`.
  */
-private class StdStringAssign extends TaintFunction {
-  StdStringAssign() { this.getClassAndName("assign") instanceof StdBasicString }
+private class StdStringInsert extends StdStringTaintFunction {
+  StdStringInsert() { this.getClassAndName("insert") instanceof StdBasicString }
 
   /**
-   * Gets the index of a parameter to this function that is a string (or
-   * character).
+   * Holds if the return type is an iterator.
    */
-  int getAStringParameterIndex() {
-    this.getParameter(result).getType() instanceof PointerType or // e.g. `std::basic_string::CharT *`
-    this.getParameter(result).getType() instanceof ReferenceType or // e.g. `std::basic_string &`
-    this.getParameter(result).getUnspecifiedType() =
-      this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. `std::basic_string::CharT`
+  predicate hasIteratorReturnValue() { this.getType() instanceof Iterator }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    // flow from string and parameter to string (qualifier) and return value
+    (
+      input.isQualifierObject() or
+      input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getAnIteratorParameterIndex())
+    ) and
+    (
+      output.isQualifierObject()
+      or
+      if this.hasIteratorReturnValue() then output.isReturnValue() else output.isReturnValueDeref()
+    )
+    or
+    // reverse flow from returned reference to the qualifier (for writes to
+    // the result)
+    not this.hasIteratorReturnValue() and
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
   }
+}
 
-  /**
-   * Gets the index of a parameter to this function that is an iterator.
-   */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
+/**
+ * The standard function `std::string.assign`.
+ */
+private class StdStringAssign extends StdStringTaintFunction {
+  StdStringAssign() { this.getClassAndName("assign") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from parameter to string itself (qualifier) and return value
@@ -229,7 +244,7 @@ private class StdStringAssign extends TaintFunction {
 /**
  * The standard function `std::string.copy`.
  */
-private class StdStringCopy extends TaintFunction {
+private class StdStringCopy extends StdStringTaintFunction {
   StdStringCopy() { this.getClassAndName("copy") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -242,7 +257,7 @@ private class StdStringCopy extends TaintFunction {
 /**
  * The standard function `std::string.substr`.
  */
-private class StdStringSubstr extends TaintFunction {
+private class StdStringSubstr extends StdStringTaintFunction {
   StdStringSubstr() { this.getClassAndName("substr") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -255,7 +270,7 @@ private class StdStringSubstr extends TaintFunction {
 /**
  * The `std::string` functions `at` and `operator[]`.
  */
-private class StdStringAt extends TaintFunction {
+private class StdStringAt extends StdStringTaintFunction {
   StdStringAt() { this.getClassAndName(["at", "operator[]"]) instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {

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

@@ -6,6 +6,8 @@ uniqueNodeToString
 missingToString
 parameterCallable
 localFlowIsLocal
+readStepIsLocal
+storeStepIsLocal
 compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes

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

@@ -21,6 +21,8 @@ uniqueNodeToString
 missingToString
 parameterCallable
 localFlowIsLocal
+readStepIsLocal
+storeStepIsLocal
 compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes

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

@@ -12,6 +12,8 @@ uniqueNodeToString
 missingToString
 parameterCallable
 localFlowIsLocal
+readStepIsLocal
+storeStepIsLocal
 compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes