Merge branch 'main' into js-followMsg

Author: erik-krogh

.github/workflows/check-qldoc.yml

@@ -27,7 +27,8 @@ jobs:
         run: |
           EXIT_CODE=0
           # TODO: remove the swift exception from the regex when we fix generated QLdoc
-          changed_lib_packs="$(git diff --name-only --diff-filter=ACMRT HEAD^ HEAD | { grep -Po '^(?!swift)[a-z]*/ql/lib' || true; } | sort -u)"
+          # TODO: remove the shared exception from the regex when coverage of qlpacks without dbschemes is supported
+          changed_lib_packs="$(git diff --name-only --diff-filter=ACMRT HEAD^ HEAD | { grep -Po '^(?!(swift|shared))[a-z]*/ql/lib' || true; } | sort -u)"
           for pack_dir in ${changed_lib_packs}; do
             lang="${pack_dir%/ql/lib}"
             codeql generate library-doc-coverage --output="${RUNNER_TEMP}/${lang}-current.txt" --dir="${pack_dir}"

.github/workflows/ruby-build.yml

@@ -95,6 +95,7 @@ jobs:
         uses: ./.github/actions/fetch-codeql
       - name: Build Query Pack
         run: |
+          codeql pack create ../shared/ssa --output target/packs
           codeql pack create ql/lib --output target/packs
           codeql pack install ql/src
           codeql pack create ql/src --output target/packs

codeql-workspace.yml

@@ -4,6 +4,7 @@ provide:
   - "*/ql/test/qlpack.yml"
   - "*/ql/examples/qlpack.yml"
   - "*/ql/consistency-queries/qlpack.yml"
+  - "shared/*/qlpack.yml"
   - "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml"
   - "go/ql/config/legacy-support/qlpack.yml"
   - "go/build/codeql-extractor-go/codeql-extractor.yml"

config/identical-files.json

@@ -461,12 +461,6 @@
     "javascript/ql/lib/IDEContextual.qll",
     "python/ql/lib/analysis/IDEContextual.qll"
   ],
-  "SSA C#": [
-    "csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImplCommon.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplCommon.qll",
-    "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll",
-    "swift/ql/lib/codeql/swift/dataflow/internal/SsaImplCommon.qll"
-  ],
   "CryptoAlgorithms Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
     "python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll",

cpp/ql/lib/change-notes/2022-09-08-implicit-read-flowstates.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* Fixed an issue in the taint tracking analysis where implicit reads were not allowed by default in sinks or additional taint steps that used flow states.

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/Bound.qll

@@ -73,7 +73,7 @@ class ValueNumberBound extends Bound, TBoundValueNumber {
     this = TBoundValueNumber(valueNumber(result)) and delta = 0
   }
 
-  override string toString() { result = vn.getExampleInstruction().toString() }
+  override string toString() { result = "ValueNumberBound" }
 
   override Location getLocation() { result = vn.getLocation() }
 

cpp/ql/lib/experimental/semmle/code/cpp/semantic/SemanticExprSpecific.qll

@@ -112,12 +112,11 @@ module SemanticExprConfig {
 
   predicate hasDominanceInformation(BasicBlock block) { any() }
 
-  int getBasicBlockUniqueId(BasicBlock block) {
-    // REVIEW: `getDisplayIndex()` is not intended for use in real queries, but for now it's the
-    // best we can do because `equivalentRelation` won't accept a predicate whose parameters are IPA
-    // types.
-    result = block.getDisplayIndex()
-  }
+  private predicate id(Cpp::Locatable x, Cpp::Locatable y) { x = y }
+
+  private predicate idOf(Cpp::Locatable x, int y) = equivalenceRelation(id/2)(x, y)
+
+  int getBasicBlockUniqueId(BasicBlock block) { idOf(block.getFirstInstruction().getAst(), result) }
 
   newtype TSsaVariable =
     TSsaInstruction(IR::Instruction instr) { instr.hasMemoryResult() } or
@@ -267,17 +266,7 @@ module SemanticExprConfig {
 
     ValueNumberBound() { bound = this }
 
-    override string toString() {
-      result =
-        min(SsaVariable v |
-          v.asInstruction() = bound.getValueNumber().getAnInstruction()
-        |
-          v
-          order by
-            v.asInstruction().getBlock().getDisplayIndex(),
-            v.asInstruction().getDisplayIndexInBlock()
-        ).toString()
-    }
+    override string toString() { result = bound.toString() }
   }
 
   predicate zeroBound(Bound bound) { bound instanceof IRBound::ZeroBound }

cpp/ql/lib/experimental/semmle/code/cpp/semantic/analysis/RangeAnalysis.qll

@@ -542,12 +542,32 @@ private predicate unequalIntegralSsa(
 ) {
   exists(SemExpr e, int d1, int d2 |
     unequalFlowStepIntegralSsa(v, pos, e, d1, reason) and
-    bounded(e, b, d2, true, _, _, _) and
-    bounded(e, b, d2, false, _, _, _) and
+    boundedUpper(e, b, d1) and
+    boundedLower(e, b, d2) and
     delta = d2 + d1
   )
 }
 
+/**
+ * Holds if `b + delta` is an upper bound for `e`.
+ *
+ * This predicate only exists to prevent a bad standard order in `unequalIntegralSsa`.
+ */
+pragma[nomagic]
+private predicate boundedUpper(SemExpr e, SemBound b, int delta) {
+  bounded(e, b, delta, true, _, _, _)
+}
+
+/**
+ * Holds if `b + delta` is a lower bound for `e`.
+ *
+ * This predicate only exists to prevent a bad standard order in `unequalIntegralSsa`.
+ */
+pragma[nomagic]
+private predicate boundedLower(SemExpr e, SemBound b, int delta) {
+  bounded(e, b, delta, false, _, _, _)
+}
+
 /** Weakens a delta to lie in the range `[-1..1]`. */
 bindingset[delta, upper]
 private int weakenDelta(boolean upper, int delta) {

cpp/ql/lib/qlpack.yml

@@ -5,3 +5,5 @@ dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
 library: true
 upgrades: upgrades
+dependencies:
+  codeql/ssa: 0.0.1

cpp/ql/lib/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -172,7 +172,12 @@ abstract class Configuration extends DataFlow::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    (
+      this.isSink(node) or
+      this.isSink(node, _) or
+      this.isAdditionalTaintStep(node, _) or
+      this.isAdditionalTaintStep(node, _, _, _)
+    ) and
     defaultImplicitTaintRead(node, c)
   }
 

cpp/ql/lib/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -172,7 +172,12 @@ abstract class Configuration extends DataFlow::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    (
+      this.isSink(node) or
+      this.isSink(node, _) or
+      this.isAdditionalTaintStep(node, _) or
+      this.isAdditionalTaintStep(node, _, _, _)
+    ) and
     defaultImplicitTaintRead(node, c)
   }
 

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

@@ -4,7 +4,7 @@ private import DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
 private import semmle.code.cpp.models.interfaces.Allocation as Alloc
 private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
-private import SsaImplCommon as SsaImplCommon
+private import codeql.ssa.Ssa as SsaImplCommon
 
 private module SourceVariables {
   private newtype TSourceVariable =

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -172,7 +172,12 @@ abstract class Configuration extends DataFlow::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    (
+      this.isSink(node) or
+      this.isSink(node, _) or
+      this.isAdditionalTaintStep(node, _) or
+      this.isAdditionalTaintStep(node, _, _, _)
+    ) and
     defaultImplicitTaintRead(node, c)
   }
 

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -172,7 +172,12 @@ abstract class Configuration extends DataFlow::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    (
+      this.isSink(node) or
+      this.isSink(node, _) or
+      this.isAdditionalTaintStep(node, _) or
+      this.isAdditionalTaintStep(node, _, _, _)
+    ) and
     defaultImplicitTaintRead(node, c)
   }
 

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

@@ -172,7 +172,12 @@ abstract class Configuration extends DataFlow::Configuration {
   }
 
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    (
+      this.isSink(node) or
+      this.isSink(node, _) or
+      this.isAdditionalTaintStep(node, _) or
+      this.isAdditionalTaintStep(node, _, _, _)
+    ) and
     defaultImplicitTaintRead(node, c)
   }