github · lcartey · Aug 1, 2023 · Jul 14, 2022 · Jul 14, 2022 · Jul 14, 2022
diff --git a/.codeqlmanifest.json b/.codeqlmanifest.json
@@ -1 +1,9 @@
-{ "provide": [ "cpp/*/src/qlpack.yml", "cpp/*/test/qlpack.yml", "c/*/src/qlpack.yml", "c/*/test/qlpack.yml", "scripts/generate_modules/queries/qlpack.yml" ] }
+{
+  "provide": [
+    "cpp/*/src/qlpack.yml",
+    "cpp/*/test/qlpack.yml",
+    "c/*/src/qlpack.yml",
+    "c/*/test/qlpack.yml",
+    "scripts/generate_modules/queries/qlpack.yml"
+  ]
+}
diff --git a/c/cert/src/codeql-pack.lock.yml b/c/cert/src/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/c/cert/src/codeql-suites/cert-default.qls b/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
diff --git a/c/cert/src/qlpack.yml b/c/cert/src/qlpack.yml
@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-c-coding-standards: '*'
-  codeql/cpp-all: 0.3.5
+  codeql/cpp-all: 0.4.6
diff --git a/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql
@@ -15,8 +15,9 @@ import cpp
 import codingstandards.c.cert
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.security.Security
-import semmle.code.cpp.security.TaintTracking
-import TaintedWithPath
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import DataFlow::PathGraph
 
 // Query TaintedPath.ql from the CodeQL standard library
 /**
@@ -45,20 +46,93 @@ class FileFunction extends FunctionWithWrappers {
   override predicate interestingArg(int arg) { arg = 0 }
 }
 
-class TaintedPathConfiguration extends TaintTrackingConfiguration {
-  override predicate isSink(Element tainted) {
-    exists(FileFunction fileFunction | fileFunction.outermostWrapperFunctionCall(tainted, _))
+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
+ * there are a lot of valid ways of doing an upper bounds checks if we don't consider
+ * where it occurs, for example:
+ * ```cpp
+ *   if (x < 10) { sink(x); }
+ *
+ *   if (10 > y) { sink(y); }
+ *
+ *   if (z > 10) { z = 10; }
+ *   sink(z);
+ * ```
+ */
+predicate hasUpperBoundsCheck(Variable var) {
+  exists(RelationalOperation oper, VariableAccess access |
+    oper.getAnOperand() = access and
+    access.getTarget() = var and
+    // Comparing to 0 is not an upper bound check
+    not oper.getAnOperand().getValue() = "0"
+  )
+}
+
+class TaintedPathConfiguration extends TaintTracking::Configuration {
+  TaintedPathConfiguration() { this = "TaintedPathConfiguration" }
+
+  override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(FileFunction fileFunction |
+      fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _)
+    )
+  }
+
+  override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
+    or
+    exists(LoadInstruction load, Variable checkedVar |
+      load = node.asInstruction() and
+      checkedVar = load.getSourceAddress().(VariableAddressInstruction).getAstVariable() and
+      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())
+    )
   }
 }
 
 from
-  FileFunction fileFunction, Expr taintedArg, Expr taintSource, PathNode sourceNode,
-  PathNode sinkNode, string taintCause, string callChain
+  FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
+  DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
 where
   not isExcluded(taintedArg, IO3Package::doNotPerformFileOperationsOnDevicesQuery()) and
+  taintedArg = asSinkExpr(sinkNode.getNode()) and
   fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
-  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
+  cfg.hasFilteredFlowPath(sourceNode, sinkNode) and
+  taintSource = asSourceExpr(sourceNode.getNode()) and
   isUserInput(taintSource, taintCause)
 select taintedArg, sourceNode, sinkNode,
-  "This argument to a file access function is derived from $@ and then passed to " + callChain,
+  "This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
   taintSource, "user input (" + taintCause + ")"
diff --git a/c/cert/test/codeql-pack.lock.yml b/c/cert/test/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected b/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected
@@ -1,40 +1,16 @@
 edges
-| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | (const char *)... |
-| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | file_name |
-| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | file_name indirection |
-| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | (const char *)... |
-| test.c:20:15:20:23 | file_name | 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 |
-| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | (const char *)... |
-| test.c:20:15:20:23 | scanf output argument | 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 |
-| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | (LPCTSTR)... |
-| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | file_name |
-| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | file_name indirection |
-| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | (LPCTSTR)... |
-| test.c:45:15:45:23 | file_name | 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 |
-| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | (LPCTSTR)... |
-| test.c:45:15:45:23 | scanf output argument | 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 |
-subpaths
 nodes
-| test.c:20:15:20:23 | array to pointer conversion | semmle.label | array to pointer conversion |
 | 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 | (const char *)... | semmle.label | (const char *)... |
-| test.c:21:8:21:16 | (const char *)... | semmle.label | (const char *)... |
-| test.c:21:8:21:16 | file_name | semmle.label | file_name |
-| test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection |
 | test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection |
-| test.c:45:15:45:23 | array to pointer conversion | semmle.label | array to pointer conversion |
 | 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 | (LPCTSTR)... | semmle.label | (LPCTSTR)... |
-| test.c:46:29:46:37 | (LPCTSTR)... | semmle.label | (LPCTSTR)... |
-| test.c:46:29:46:37 | file_name | semmle.label | file_name |
-| test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection |
 | 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 | 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 | 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 | 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) |
diff --git a/c/common/src/codeql-pack.lock.yml b/c/common/src/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/c/common/src/codingstandards/c/Pointers.qll b/c/common/src/codingstandards/c/Pointers.qll
@@ -60,7 +60,7 @@ class ArrayPointerArithmeticExpr extends PointerArithmeticExpr, ArrayExpr {
  * A null pointer constant, which is either in the form `NULL` or `(void *)0`.
  */
 predicate isNullPointerConstant(Expr e) {
-  e.findRootCause() instanceof NULLMacro
+  e.findRootCause() instanceof NullMacro
   or
   exists(CStyleCast c |
     not c.isImplicit() and

diff --git a/c/common/src/qlpack.yml b/c/common/src/qlpack.yml
@@ -3,4 +3,4 @@ version: 2.19.0-dev
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'
-  codeql/cpp-all: 0.3.5
+  codeql/cpp-all: 0.4.6
diff --git a/c/common/test/codeql-pack.lock.yml b/c/common/test/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/c/misra/src/codeql-pack.lock.yml b/c/misra/src/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/c/misra/src/codeql-suites/misra-default.qls b/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
diff --git a/c/misra/src/qlpack.yml b/c/misra/src/qlpack.yml
@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-c-coding-standards: '*'
-  codeql/cpp-all: 0.3.5
+  codeql/cpp-all: 0.4.6
diff --git a/c/misra/test/codeql-pack.lock.yml b/c/misra/test/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/change_notes/2022-12-06-remove-use-of-default-taint-tracking.md b/change_notes/2022-12-06-remove-use-of-default-taint-tracking.md
@@ -0,0 +1,2 @@
+ - `FIO32-C` - `DoNotPerformFileOperationsOnDevices.ql`:
+   - The query was rewritten to no longer depend of the `DefaultTaintTracking` library, which will be deprecated.
diff --git a/change_notes/2023-07-30-update-to-2.11.6.md b/change_notes/2023-07-30-update-to-2.11.6.md
@@ -0,0 +1 @@
+ - Updated the supported CodeQL version to `2.11.6`.
diff --git a/cpp/autosar/src/codeql-pack.lock.yml b/cpp/autosar/src/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/cpp/autosar/src/codeql-suites/autosar-advisory.qls b/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
diff --git a/cpp/autosar/src/codeql-suites/autosar-audit.qls b/cpp/autosar/src/codeql-suites/autosar-audit.qls
@@ -5,4 +5,4 @@
     - problem
     - path-problem
     tags contain:
-    - external/autosar/audit
+    - external/autosar/audit
diff --git a/cpp/autosar/src/codeql-suites/autosar-default.qls b/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
diff --git a/cpp/autosar/src/codeql-suites/autosar-required.qls b/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
diff --git a/cpp/autosar/src/codeql-suites/autosar-single-translation-unit.qls b/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
diff --git a/cpp/autosar/src/codingstandards/cpp/HardwareOrProtocolInterface.qll b/cpp/autosar/src/codingstandards/cpp/HardwareOrProtocolInterface.qll
@@ -39,7 +39,7 @@ class DefinedSizeType extends Type {
 
 class DefinedSizeClass extends Class {
   DefinedSizeClass() {
-    this.isPOD() and
+    this.isPod() and
     forall(Field f | f = this.getAField() | f.getType() instanceof DefinedSizeType)
   }
 }
diff --git a/cpp/autosar/src/qlpack.yml b/cpp/autosar/src/qlpack.yml
@@ -5,4 +5,4 @@ suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'
-  codeql/cpp-all: 0.3.5
+  codeql/cpp-all: 0.4.6
diff --git a/cpp/autosar/src/rules/A11-0-1/NonPodTypeShouldBeDefinedAsClass.ql b/cpp/autosar/src/rules/A11-0-1/NonPodTypeShouldBeDefinedAsClass.ql
@@ -22,5 +22,5 @@ import codingstandards.cpp.Typehelpers
 from Struct s
 where
   not isExcluded(s, ClassesPackage::nonPodTypeShouldBeDefinedAsClassQuery()) and
-  not s.isPOD()
+  not s.isPod()
 select s, "Non-POD type defined as struct instead of class."
diff --git a/cpp/autosar/src/rules/A12-0-2/OperationsAssumingMemoryLayoutPerformedOnObjects.ql b/cpp/autosar/src/rules/A12-0-2/OperationsAssumingMemoryLayoutPerformedOnObjects.ql
@@ -19,7 +19,7 @@ import cpp
 import codingstandards.cpp.autosar
 
 class Object extends Class {
-  Object() { not this.(Struct).isPOD() }
+  Object() { not this.(Struct).isPod() }
 }
 
 predicate isPointerToObject(Expr e) {

diff --git a/...9-6-1/DataTypesUsedForInterfacingWithHardwareOrProtocolsMustBeTrivialAndStandardLayout.ql b/...9-6-1/DataTypesUsedForInterfacingWithHardwareOrProtocolsMustBeTrivialAndStandardLayout.ql
@@ -23,6 +23,6 @@ from HardwareOrProtocolInterfaceClass c
 where
   not isExcluded(c,
     ClassesPackage::dataTypesUsedForInterfacingWithHardwareOrProtocolsMustBeTrivialAndStandardLayoutQuery()) and
-  not c.isPOD()
+  not c.isPod()
 select c,
   "Data type used for hardware interface or communication protocol is not standard layout and trivial."
diff --git a/cpp/autosar/src/rules/M11-0-1/MemberDataInNonPodClassTypesNotPrivate.ql b/cpp/autosar/src/rules/M11-0-1/MemberDataInNonPodClassTypesNotPrivate.ql
@@ -17,7 +17,7 @@ import cpp
 import codingstandards.cpp.autosar
 
 class NonPODType extends Class {
-  NonPODType() { not this.isPOD() }
+  NonPODType() { not this.isPod() }
 }
 
 from NonPODType p, Field f

diff --git a/cpp/autosar/test/codeql-pack.lock.yml b/cpp/autosar/test/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
diff --git a/cpp/cert/src/codeql-pack.lock.yml b/cpp/cert/src/codeql-pack.lock.yml
@@ -1,6 +1,8 @@
 ---
+lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 0.3.5
+    version: 0.4.6
+  codeql/ssa:
+    version: 0.0.7
 compiled: false
-lockVersion: 1.0.0
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		- `FIO32-C` - `DoNotPerformFileOperationsOnDevices.ql`:
		- The query was rewritten to no longer depend of the `DefaultTaintTracking` library, which will be deprecated.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		- Updated the supported CodeQL version to `2.11.6`.