Merge branch 'main' into explicit-this

rcowsill · Nov 24, 2021 · 08ce03c · 08ce03c
2 parents f0c5a80 + 3bab8c6
commit 08ce03c
Show file tree

Hide file tree

Showing 455 changed files with 22,622 additions and 4,212 deletions.
diff --git a/.codeqlmanifest.json b/.codeqlmanifest.json
@@ -1,11 +1,16 @@
-{ "provide": [ "ruby/.codeqlmanifest.json",
-                "*/ql/src/qlpack.yml",
-               "*/ql/lib/qlpack.yml",
-               "*/ql/test/qlpack.yml",
-               "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
-               "*/ql/examples/qlpack.yml",
-               "*/upgrades/qlpack.yml",
-               "javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
-               "javascript/ql/experimental/adaptivethreatmodeling/src/qlpack.yml",
-               "misc/legacy-support/*/qlpack.yml",
-               "misc/suite-helpers/qlpack.yml" ] }
+{
+    "provide": [
+        "*/ql/src/qlpack.yml",
+        "*/ql/lib/qlpack.yml",
+        "*/ql/test/qlpack.yml",
+        "*/ql/examples/qlpack.yml",
+        "*/upgrades/qlpack.yml",
+        "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
+        "javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
+        "javascript/ql/experimental/adaptivethreatmodeling/src/qlpack.yml",
+        "misc/legacy-support/*/qlpack.yml",
+        "misc/suite-helpers/qlpack.yml",
+        "ruby/ql/consistency-queries/qlpack.yml",
+        "ruby/extractor-pack/codeql-extractor.yml"
+   ]
+}
diff --git a/.github/workflows/ruby-build.yml b/.github/workflows/ruby-build.yml
@@ -102,16 +102,6 @@ jobs:
           PACK_FOLDER=$(readlink -f target/packs/codeql/ruby-queries/*)
           codeql/codeql generate query-help --format=sarifv2.1.0 --output="${PACK_FOLDER}/rules.sarif" ql/src
           (cd ql/src; find queries \( -name '*.qhelp' -o -name '*.rb' -o -name '*.erb' \) -exec bash -c 'mkdir -p "'"${PACK_FOLDER}"'/$(dirname "{}")"' \; -exec cp "{}" "${PACK_FOLDER}/{}" \;)
-      - name: Compile with previous CodeQL versions
-        run: |
-          for version in  $(gh release list --repo https://github.com/github/codeql-cli-binaries | cut -f 1 | sort --version-sort | tail -3 | head -2); do
-            rm -f codeql-linux64.zip
-            gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip "$version"
-            rm -rf codeql; unzip -q codeql-linux64.zip
-            codeql/codeql query compile target/packs/*
-          done
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
       - uses: actions/upload-artifact@v2
         with:
           name: codeql-ruby-queries

diff --git a/.github/workflows/ruby-qltest.yml b/.github/workflows/ruby-qltest.yml
@@ -32,7 +32,7 @@ jobs:
       - uses: ./ruby/actions/create-extractor-pack
       - name: Run QL tests
         run: |
-          codeql test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ruby" --additional-packs "${{ github.workspace }}"  --consistency-queries ql/consistency-queries ql/test
+          codeql test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Check QL formatting

diff --git a/.gitignore b/.gitignore
@@ -27,3 +27,6 @@ csharp/extractor/Semmle.Extraction.CSharp.Driver/Properties/launchSettings.json
 
 # Avoid committing cached package components
 .codeql
+
+# Compiled class file
+*.class
diff --git a/config/identical-files.json b/config/identical-files.json
@@ -460,9 +460,10 @@
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
     "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll"
   ],
-  "ReDoS Util Python/JS": [
+  "ReDoS Util Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/performance/ReDoSUtil.qll",
-    "python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll"
+    "python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll",
+    "ruby/ql/lib/codeql/ruby/security/performance/ReDoSUtil.qll"
   ],
   "ReDoS Exponential Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/performance/ExponentialBackTracking.qll",
@@ -471,7 +472,12 @@
   "ReDoS Polynomial Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/performance/SuperlinearBackTracking.qll",
     "python/ql/lib/semmle/python/security/performance/SuperlinearBackTracking.qll",
-    "ruby/ql/lib/codeql/ruby/regexp/SuperlinearBackTracking.qll"
+    "ruby/ql/lib/codeql/ruby/security/performance/SuperlinearBackTracking.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"
   ],
   "CFG": [
     "csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImplShared.qll",

diff --git a/cpp/change-notes/2021-11-09-use-of-http.md b/cpp/change-notes/2021-11-09-use-of-http.md
@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.
diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Printf.qll
@@ -6,6 +6,8 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.commons.CommonType
 import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
+private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
@@ -268,6 +270,18 @@ class FormattingFunctionCall extends Expr {
   }
 }
 
+/**
+ * Gets the number of digits required to represent the integer represented by `f`.
+ *
+ * `f` is assumed to be nonnegative.
+ */
+bindingset[f]
+private int lengthInBase10(float f) {
+  f = 0 and result = 1
+  or
+  result = f.log10().floor() + 1
+}
+
 /**
  * A class to represent format strings that occur as arguments to invocations of formatting functions.
  */
@@ -1046,39 +1060,63 @@ class FormatLiteral extends Literal {
         or
         this.getConversionChar(n).toLowerCase() = ["d", "i"] and
         // e.g. -2^31 = "-2147483648"
-        exists(int sizeBits |
-          sizeBits =
-            min(int bits |
-              bits = this.getIntegralDisplayType(n).getSize() * 8
-              or
-              exists(IntegralType t |
-                t = this.getUse().getConversionArgument(n).getType().getUnderlyingType()
-              |
-                t.isSigned() and bits = t.getSize() * 8
-              )
-            ) and
-          len = 1 + ((sizeBits - 1) / 10.0.log2()).ceil()
-          // this calculation is as %u (below) only we take out the sign bit (- 1) and allow a whole
-          // character for it to be expressed as '-'.
-        )
+        len =
+          min(float cand |
+            // The first case handles length sub-specifiers
+            // Subtract one in the exponent because one bit is for the sign.
+            // Add 1 to account for the possible sign in the output.
+            cand = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1))
+            or
+            // The second case uses range analysis to deduce a length that's shorter than the length
+            // of the number -2^31.
+            exists(Expr arg, float lower, float upper |
+              arg = this.getUse().getConversionArgument(n) and
+              lower = lowerBound(arg.getFullyConverted()) and
+              upper = upperBound(arg.getFullyConverted())
+            |
+              cand =
+                max(int cand0 |
+                  // Include the sign bit in the length if it can be negative
+                  (
+                    if lower < 0
+                    then cand0 = 1 + lengthInBase10(lower.abs())
+                    else cand0 = lengthInBase10(lower)
+                  )
+                  or
+                  (
+                    if upper < 0
+                    then cand0 = 1 + lengthInBase10(upper.abs())
+                    else cand0 = lengthInBase10(upper)
+                  )
+                )
+            )
+          )
         or
         this.getConversionChar(n).toLowerCase() = "u" and
         // e.g. 2^32 - 1 = "4294967295"
-        exists(int sizeBits |
-          sizeBits =
-            min(int bits |
-              bits = this.getIntegralDisplayType(n).getSize() * 8
-              or
-              exists(IntegralType t |
-                t = this.getUse().getConversionArgument(n).getType().getUnderlyingType()
-              |
-                t.isUnsigned() and bits = t.getSize() * 8
-              )
-            ) and
-          len = (sizeBits / 10.0.log2()).ceil()
-          // convert the size from bits to decimal characters, and round up as you can't have
-          // fractional characters (10.0.log2() is the number of bits expressed per decimal character)
-        )
+        len =
+          min(float cand |
+            // The first case handles length sub-specifiers
+            cand = 2.pow(this.getIntegralDisplayType(n).getSize() * 8)
+            or
+            // The second case uses range analysis to deduce a length that's shorter than
+            // the length of the number 2^31 - 1.
+            exists(Expr arg, float lower |
+              arg = this.getUse().getConversionArgument(n) and
+              lower = lowerBound(arg.getFullyConverted())
+            |
+              cand =
+                max(float cand0 |
+                  // If lower can be negative we use `(unsigned)-1` as the candidate value.
+                  lower < 0 and
+                  cand0 = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
+                  or
+                  cand0 = upperBound(arg.getFullyConverted())
+                )
+            )
+          |
+            lengthInBase10(cand)
+          )
         or
         this.getConversionChar(n).toLowerCase() = "x" and
         // e.g. "12345678"

diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll
@@ -3740,13 +3740,14 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
+      not arg.isHidden()
     )
   }
 
@@ -3780,8 +3781,17 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp() and
-      not ret.isHidden()
+      apout = ret.getAp()
+    )
+  }
+
+  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
+    n.getASuccessorImpl() = result and
+    result.isHidden() and
+    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
+      localFlowBigStep(n1, n2, _, _, _, _) or
+      store(n1, _, n2, _, _) or
+      read(n1, _, n2, _)
     )
   }
 
@@ -3790,11 +3800,12 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, ret, o, apout) and
+      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
+      not ret.isHidden() and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll
@@ -3740,13 +3740,14 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
+      not arg.isHidden()
     )
   }
 
@@ -3780,8 +3781,17 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp() and
-      not ret.isHidden()
+      apout = ret.getAp()
+    )
+  }
+
+  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
+    n.getASuccessorImpl() = result and
+    result.isHidden() and
+    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
+      localFlowBigStep(n1, n2, _, _, _, _) or
+      store(n1, _, n2, _, _) or
+      read(n1, _, n2, _)
     )
   }
 
@@ -3790,11 +3800,12 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, ret, o, apout) and
+      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
+      not ret.isHidden() and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll
@@ -3740,13 +3740,14 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
+      not arg.isHidden()
     )
   }
 
@@ -3780,8 +3781,17 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp() and
-      not ret.isHidden()
+      apout = ret.getAp()
+    )
+  }
+
+  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
+    n.getASuccessorImpl() = result and
+    result.isHidden() and
+    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
+      localFlowBigStep(n1, n2, _, _, _, _) or
+      store(n1, _, n2, _, _) or
+      read(n1, _, n2, _)
     )
   }
 
@@ -3790,11 +3800,12 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, ret, o, apout) and
+      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
+      not ret.isHidden() and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout
-Original file line number
+Diff line change
@@ Expand Up @@
     # Avoid committing cached package components
     .codeql
+    # Compiled class file
+    *.class
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		lgtm,codescanning
		* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.