Merge pull request #343 from github/lcartey/update-to-2.12

Update CodeQL CLI dependency to 2.12.7.

Author: jsinglet

c/cert/src/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

c/cert/src/codeql-suites/cert-default.qls

@@ -6,4 +6,4 @@
     - path-problem
 - exclude:
     tags contain:
-    - external/cert/default-disabled
+    - external/cert/default-disabled

c/cert/src/qlpack.yml

@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-c-coding-standards: '*'
-  codeql/cpp-all: 0.4.6
+  codeql/cpp-all: 0.6.1

c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql

@@ -14,10 +14,10 @@
 import cpp
 import codingstandards.c.cert
 import semmle.code.cpp.security.FunctionWithWrappers
-import semmle.code.cpp.security.Security
+import semmle.code.cpp.security.FlowSources
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.ir.dataflow.TaintTracking
-import DataFlow::PathGraph
+import TaintedPath::PathGraph
 
 // Query TaintedPath.ql from the CodeQL standard library
 /**
@@ -46,22 +46,6 @@ class FileFunction extends FunctionWithWrappers {
   override predicate interestingArg(int arg) { arg = 0 }
 }
 
-Expr asSourceExpr(DataFlow::Node node) {
-  result = node.asConvertedExpr()
-  or
-  result = node.asDefiningArgument()
-}
-
-Expr asSinkExpr(DataFlow::Node node) {
-  result =
-    node.asOperand()
-        .(SideEffectOperand)
-        .getUse()
-        .(ReadSideEffectInstruction)
-        .getArgumentDef()
-        .getUnconvertedResultExpression()
-}
-
 /**
  * Holds for a variable that has any kind of upper-bound check anywhere in the program.
  * This is biased towards being inclusive and being a coarse overapproximation because
@@ -85,20 +69,16 @@ predicate hasUpperBoundsCheck(Variable var) {
   )
 }
 
-class TaintedPathConfiguration extends TaintTracking::Configuration {
-  TaintedPathConfiguration() { this = "TaintedPathConfiguration" }
-
-  override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
+module TaintedPathConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
 
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     exists(FileFunction fileFunction |
-      fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _)
+      fileFunction.outermostWrapperFunctionCall(node.asIndirectArgument(), _)
     )
   }
 
-  override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
-
-  override predicate isSanitizer(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
     or
     exists(LoadInstruction load, Variable checkedVar |
@@ -107,32 +87,19 @@ class TaintedPathConfiguration extends TaintTracking::Configuration {
       hasUpperBoundsCheck(checkedVar)
     )
   }
-
-  predicate hasFilteredFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
-    this.hasFlowPath(source, sink) and
-    // The use of `isUserInput` in `isSink` in combination with `asSourceExpr` causes
-    // duplicate results. Filter these duplicates. The proper solution is to switch to
-    // using `LocalFlowSource` and `RemoteFlowSource`, but this currently only supports
-    // a subset of the cases supported by `isUserInput`.
-    not exists(DataFlow::PathNode source2 |
-      this.hasFlowPath(source2, sink) and
-      asSourceExpr(source.getNode()) = asSourceExpr(source2.getNode())
-    |
-      not exists(source.getNode().asConvertedExpr()) and exists(source2.getNode().asConvertedExpr())
-    )
-  }
 }
 
+module TaintedPath = TaintTracking::Make<TaintedPathConfiguration>;
+
 from
-  FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
-  DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
+  FileFunction fileFunction, Expr taintedArg, FlowSource taintSource,
+  TaintedPath::PathNode sourceNode, TaintedPath::PathNode sinkNode, string callChain
 where
   not isExcluded(taintedArg, IO3Package::doNotPerformFileOperationsOnDevicesQuery()) and
-  taintedArg = asSinkExpr(sinkNode.getNode()) and
+  taintedArg = sinkNode.getNode().asIndirectArgument() and
   fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
-  cfg.hasFilteredFlowPath(sourceNode, sinkNode) and
-  taintSource = asSourceExpr(sourceNode.getNode()) and
-  isUserInput(taintSource, taintCause)
+  TaintedPath::hasFlowPath(sourceNode, sinkNode) and
+  taintSource = sourceNode.getNode()
 select taintedArg, sourceNode, sinkNode,
   "This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
-  taintSource, "user input (" + taintCause + ")"
+  taintSource, "user input (" + taintSource.getSourceType() + ")"

c/cert/test/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected

@@ -1,16 +1,12 @@
 edges
-| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection |
 | test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection |
-| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection |
 | test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection |
 nodes
-| test.c:20:15:20:23 | file_name | semmle.label | file_name |
 | test.c:20:15:20:23 | scanf output argument | semmle.label | scanf output argument |
 | test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection |
-| test.c:45:15:45:23 | file_name | semmle.label | file_name |
 | test.c:45:15:45:23 | scanf output argument | semmle.label | scanf output argument |
 | test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection |
 subpaths
 #select
-| test.c:21:8:21:16 | file_name | test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection | This argument to a file access function is derived from $@ and then passed to func(file_name), which calls fopen((unnamed parameter 0)). | test.c:20:15:20:23 | file_name | user input (scanf) |
-| test.c:46:29:46:37 | file_name | test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection | This argument to a file access function is derived from $@ and then passed to CreateFile(lpFileName). | test.c:45:15:45:23 | file_name | user input (scanf) |
+| test.c:21:8:21:16 | file_name | test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection | This argument to a file access function is derived from $@ and then passed to func(file_name), which calls fopen((unnamed parameter 0)). | test.c:20:15:20:23 | scanf output argument | user input (value read by scanf) |
+| test.c:46:29:46:37 | file_name | test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection | This argument to a file access function is derived from $@ and then passed to CreateFile(lpFileName). | test.c:45:15:45:23 | scanf output argument | user input (value read by scanf) |

c/common/src/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

c/common/src/codingstandards/c/OutOfBounds.qll

@@ -712,7 +712,8 @@ module OOB {
   }
 
   private class DynamicAllocationSource extends PointerToObjectSource instanceof AllocationExpr,
-    FunctionCall {
+    FunctionCall
+  {
     DynamicAllocationSource() {
       // exclude OperatorNewAllocationFunction to only deal with raw malloc-style calls,
       // which do not apply a multiple to the size of the allocation passed to them.
@@ -905,7 +906,8 @@ module OOB {
     override predicate isNotNullTerminated() { none() }
   }
 
-  private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration {
+  private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration
+  {
     PointerToObjectSourceOrSizeToBufferAccessFunctionConfig() {
       this = "PointerToObjectSourceOrSizeToBufferAccessFunctionConfig"
     }

c/common/src/qlpack.yml

@@ -3,4 +3,4 @@ version: 2.22.0-dev
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'
-  codeql/cpp-all: 0.4.6
+  codeql/cpp-all: 0.6.1

c/common/test/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

c/misra/src/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

c/misra/src/codeql-suites/misra-default.qls

@@ -7,4 +7,4 @@
 - exclude:
     tags contain:
     - external/misra/audit
-    - external/misra/default-disabled
+    - external/misra/default-disabled

c/misra/src/qlpack.yml

@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-c-coding-standards: '*'
-  codeql/cpp-all: 0.4.6
+  codeql/cpp-all: 0.6.1

c/misra/test/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

change_notes/2022-03-16-update-for-dataflow-changes.md

@@ -0,0 +1,2 @@
+ - `FIO32-C` - `DoNotPerformFileOperationsOnDevices.ql`:
+   - The query was updated to work with the latest version of the dataflow library.

change_notes/2023-08-16-update-to-2.12.7.md

@@ -0,0 +1 @@
+ - Updated the supported CodeQL version to `2.12.7`.

change_notes/2023-08-30-a15-2-2-no-zero-paths.md

@@ -0,0 +1 @@
+ - `A15-2-2` - all results now include an associated exception flow path to avoid a CodeQL CLI bug in 2.12.7. This includes results where an exception is thrown directly in the constructor.

cpp/autosar/src/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false

cpp/autosar/src/codeql-suites/autosar-advisory.qls

@@ -8,4 +8,4 @@
     - external/autosar/obligation/advisory
 - exclude:
     tags contain:
-    - external/autosar/audit
+    - external/autosar/audit

cpp/autosar/src/codeql-suites/autosar-audit.qls

@@ -5,4 +5,4 @@
     - problem
     - path-problem
     tags contain:
-    - external/autosar/audit
+    - external/autosar/audit

cpp/autosar/src/codeql-suites/autosar-default.qls

@@ -7,4 +7,4 @@
 - exclude:
     tags contain:
     - external/autosar/audit
-    - external/autosar/default-disabled
+    - external/autosar/default-disabled

cpp/autosar/src/codeql-suites/autosar-required.qls

@@ -8,4 +8,4 @@
     - external/autosar/obligation/required
 - exclude:
     tags contain:
-    - external/autosar/audit
+    - external/autosar/audit

cpp/autosar/src/codeql-suites/autosar-single-translation-unit.qls

@@ -9,4 +9,4 @@
 - exclude:
     tags contain:
     - external/autosar/audit
-    - external/autosar/default-disabled
+    - external/autosar/default-disabled

cpp/autosar/src/qlpack.yml

@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'
-  codeql/cpp-all: 0.4.6
+  codeql/cpp-all: 0.6.1

cpp/autosar/src/rules/A15-2-2/ConstructorErrorLeavesObjectInInvalidState.ql

@@ -53,10 +53,13 @@ class NewWrapperFunction extends Function {
 
 /** An expression on which `delete` is called, directly or indirectly. */
 class DeletedExpr extends Expr {
+  pragma[noinline, nomagic]
   DeletedExpr() {
-    this = any(DeleteExpr deleteExpr).getExpr() or
+    this = any(DeleteExpr deleteExpr).getExpr()
+    or
     exists(DeleteWrapperFunction dwf, FunctionCall call |
-      this = call.getArgument(dwf.getADeleteParameter().getIndex())
+      this = call.getArgument(dwf.getADeleteParameter().getIndex()) and
+      call.getTarget() = dwf
     )
   }
 }
@@ -75,6 +78,14 @@ class DeleteWrapperFunction extends Function {
   Parameter getADeleteParameter() { result = p }
 }
 
+class ExceptionThrowingConstructor extends ExceptionThrowingFunction, Constructor {
+  ExceptionThrowingConstructor() {
+    exists(getAFunctionThrownType(this, _)) and
+    // The constructor is within the users source code
+    exists(getFile().getRelativePath())
+  }
+}
+
 class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
   Constructor c;
 
@@ -87,24 +98,20 @@ class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
   Constructor getConstructor() { result = c }
 }
 
-/**
- * Add the `nodes` predicate to ensure results with an empty path are still reported.
- */
-query predicate nodes(ExceptionFlowNode node) { any() }
-
 from
-  Constructor c, ExceptionThrownInConstructor throwingExpr, NewAllocationExpr newExpr,
-  ExceptionFlowNode exceptionSource, ExceptionFlowNode functionNode
+  ExceptionThrowingConstructor c, ExceptionThrownInConstructor throwingExpr,
+  NewAllocationExpr newExpr, ExceptionFlowNode exceptionSource,
+  ExceptionFlowNode throwingExprFlowNode, ExceptionFlowNode reportingNode
 where
   not isExcluded(c, Exceptions2Package::constructorErrorLeavesObjectInInvalidStateQuery()) and
   not isNoExceptTrue(c) and
   // Constructor must exit with an exception
   c = throwingExpr.getConstructor() and
-  throwingExpr.hasExceptionFlowReflexive(exceptionSource, functionNode, _) and
+  throwingExpr.hasExceptionFlowReflexive(exceptionSource, throwingExprFlowNode, _) and
   exists(ExceptionFlowNode mid |
     edges*(exceptionSource, mid) and
     newExpr.getASuccessor+() = mid.asThrowingExpr() and
-    edges*(mid, functionNode) and
+    edges*(mid, throwingExprFlowNode) and
     not exists(ExceptionFlowNode prior | edges(prior, mid) |
       prior.asCatchBlock().getEnclosingFunction() = c
     )
@@ -123,7 +130,16 @@ where
     DataFlow::localFlow(DataFlow::exprNode(newExpr), DataFlow::exprNode(deletedExpr)) and
     newExpr.getASuccessor+() = deletedExpr and
     deletedExpr.getASuccessor+() = throwingExpr
-  )
-select c, exceptionSource, functionNode, "Constructor throws $@ and allocates memory at $@",
+  ) and
+  // In CodeQL CLI 2.12.7 there is a bug which causes an infinite loop during results interpretation
+  // when a result includes more than maxPaths paths and also includes a path with no edges i.e.
+  // where the source and sink node are the same.
+  // To avoid this edge case, if we report a path where the source and sink are the same (i.e the
+  // throwingExpr directly throws an exception), we adjust the sink node to report the constructor,
+  // which creates a one step path from the throwingExprFlowNode to the constructor node.
+  if throwingExprFlowNode = exceptionSource
+  then reportingNode.asFunction() = c and edges(throwingExprFlowNode, reportingNode)
+  else reportingNode = throwingExprFlowNode
+select c, exceptionSource, reportingNode, "Constructor throws $@ and allocates memory at $@",
   throwingExpr, throwingExpr.(ThrowingExpr).getAnExceptionType().getExceptionName(), newExpr,
   "alloc"

cpp/autosar/test/codeql-pack.lock.yml

@@ -2,7 +2,9 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.4.6
+    version: 0.6.1
   codeql/ssa:
+    version: 0.0.14
+  codeql/tutorial:
     version: 0.0.7
 compiled: false