Merge branch 'main' into jb1/16-cryptography-models-libraries-and-queries-migration

cpp/ql/lib/semmle/code/cpp/File.qll

@@ -5,155 +5,35 @@
 import semmle.code.cpp.Element
 import semmle.code.cpp.Declaration
 import semmle.code.cpp.metrics.MetricFile
+private import codeql.util.FileSystem
+
+private module Input implements InputSig {
+  abstract class ContainerBase extends @container {
+    abstract string getAbsolutePath();
+
+    ContainerBase getParentContainer() {
+      containerparent(unresolveElement(result), underlyingElement(this))
+    }
+
+    string toString() { result = this.getAbsolutePath() }
+  }
+
+  class FolderBase extends ContainerBase, @folder {
+    override string getAbsolutePath() { folders(underlyingElement(this), result) }
+  }
+
+  class FileBase extends ContainerBase, @file {
+    override string getAbsolutePath() { files(underlyingElement(this), result) }
+  }
+
+  predicate hasSourceLocationPrefix = sourceLocationPrefix/1;
+}
+
+private module Impl = Make<Input>;
 
 /** A file or folder. */
-class Container extends Locatable, @container {
-  /**
-   * Gets the absolute, canonical path of this container, using forward slashes
-   * as path separator.
-   *
-   * The path starts with a _root prefix_ followed by zero or more _path
-   * segments_ separated by forward slashes.
-   *
-   * The root prefix is of one of the following forms:
-   *
-   *   1. A single forward slash `/` (Unix-style)
-   *   2. An upper-case drive letter followed by a colon and a forward slash,
-   *      such as `C:/` (Windows-style)
-   *   3. Two forward slashes, a computer name, and then another forward slash,
-   *      such as `//FileServer/` (UNC-style)
-   *
-   * Path segments are never empty (that is, absolute paths never contain two
-   * contiguous slashes, except as part of a UNC-style root prefix). Also, path
-   * segments never contain forward slashes, and no path segment is of the
-   * form `.` (one dot) or `..` (two dots).
-   *
-   * Note that an absolute path never ends with a forward slash, except if it is
-   * a bare root prefix, that is, the path has no path segments. A container
-   * whose absolute path has no segments is always a `Folder`, not a `File`.
-   */
-  string getAbsolutePath() { none() } // overridden by subclasses
-
-  /**
-   * Gets the relative path of this file or folder from the root folder of the
-   * analyzed source location. The relative path of the root folder itself is
-   * the empty string.
-   *
-   * This has no result if the container is outside the source root, that is,
-   * if the root folder is not a reflexive, transitive parent of this container.
-   */
-  string getRelativePath() {
-    exists(string absPath, string pref |
-      absPath = this.getAbsolutePath() and sourceLocationPrefix(pref)
-    |
-      absPath = pref and result = ""
-      or
-      absPath = pref.regexpReplaceAll("/$", "") + "/" + result and
-      not result.matches("/%")
-    )
-  }
-
-  /**
-   * Gets the base name of this container including extension, that is, the last
-   * segment of its absolute path, or the empty string if it has no segments.
-   *
-   * Here are some examples of absolute paths and the corresponding base names
-   * (surrounded with quotes to avoid ambiguity):
-   *
-   * <table border="1">
-   * <tr><th>Absolute path</th><th>Base name</th></tr>
-   * <tr><td>"/tmp/tst.js"</td><td>"tst.js"</td></tr>
-   * <tr><td>"C:/Program Files (x86)"</td><td>"Program Files (x86)"</td></tr>
-   * <tr><td>"/"</td><td>""</td></tr>
-   * <tr><td>"C:/"</td><td>""</td></tr>
-   * <tr><td>"D:/"</td><td>""</td></tr>
-   * <tr><td>"//FileServer/"</td><td>""</td></tr>
-   * </table>
-   */
-  string getBaseName() {
-    result = this.getAbsolutePath().regexpCapture(".*/(([^/]*?)(?:\\.([^.]*))?)", 1)
-  }
-
-  /**
-   * Gets the extension of this container, that is, the suffix of its base name
-   * after the last dot character, if any.
-   *
-   * In particular,
-   *
-   *  - if the name does not include a dot, there is no extension, so this
-   *    predicate has no result;
-   *  - if the name ends in a dot, the extension is the empty string;
-   *  - if the name contains multiple dots, the extension follows the last dot.
-   *
-   * Here are some examples of absolute paths and the corresponding extensions
-   * (surrounded with quotes to avoid ambiguity):
-   *
-   * <table border="1">
-   * <tr><th>Absolute path</th><th>Extension</th></tr>
-   * <tr><td>"/tmp/tst.js"</td><td>"js"</td></tr>
-   * <tr><td>"/tmp/.classpath"</td><td>"classpath"</td></tr>
-   * <tr><td>"/bin/bash"</td><td>not defined</td></tr>
-   * <tr><td>"/tmp/tst2."</td><td>""</td></tr>
-   * <tr><td>"/tmp/x.tar.gz"</td><td>"gz"</td></tr>
-   * </table>
-   */
-  string getExtension() {
-    result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(\\.([^.]*))?", 3)
-  }
-
-  /**
-   * Gets the stem of this container, that is, the prefix of its base name up to
-   * (but not including) the last dot character if there is one, or the entire
-   * base name if there is not.
-   *
-   * Here are some examples of absolute paths and the corresponding stems
-   * (surrounded with quotes to avoid ambiguity):
-   *
-   * <table border="1">
-   * <tr><th>Absolute path</th><th>Stem</th></tr>
-   * <tr><td>"/tmp/tst.js"</td><td>"tst"</td></tr>
-   * <tr><td>"/tmp/.classpath"</td><td>""</td></tr>
-   * <tr><td>"/bin/bash"</td><td>"bash"</td></tr>
-   * <tr><td>"/tmp/tst2."</td><td>"tst2"</td></tr>
-   * <tr><td>"/tmp/x.tar.gz"</td><td>"x.tar"</td></tr>
-   * </table>
-   */
-  string getStem() {
-    result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(?:\\.([^.]*))?", 1)
-  }
-
-  /** Gets the parent container of this file or folder, if any. */
-  Container getParentContainer() {
-    containerparent(unresolveElement(result), underlyingElement(this))
-  }
-
-  /** Gets a file or sub-folder in this container. */
-  Container getAChildContainer() { this = result.getParentContainer() }
-
-  /** Gets a file in this container. */
-  File getAFile() { result = this.getAChildContainer() }
-
-  /** Gets the file in this container that has the given `baseName`, if any. */
-  File getFile(string baseName) {
-    result = this.getAFile() and
-    result.getBaseName() = baseName
-  }
-
-  /** Gets a sub-folder in this container. */
-  Folder getAFolder() { result = this.getAChildContainer() }
-
-  /** Gets the sub-folder in this container that has the given `baseName`, if any. */
-  Folder getFolder(string baseName) {
-    result = this.getAFolder() and
-    result.getBaseName() = baseName
-  }
-
-  /**
-   * Gets a textual representation of the path of this container.
-   *
-   * This is the absolute path of the container.
-   */
-  override string toString() { result = this.getAbsolutePath() }
+class Container extends Locatable, Impl::Container {
+  override string toString() { result = Impl::Container.super.toString() }
 }
 
 /**
@@ -166,9 +46,7 @@ class Container extends Locatable, @container {
  *
  * To get the full path, use `getAbsolutePath`.
  */
-class Folder extends Container, @folder {
-  override string getAbsolutePath() { folders(underlyingElement(this), result) }
-
+class Folder extends Container, Impl::Folder {
   override Location getLocation() {
     result.getContainer() = this and
     result.hasLocationInfo(_, 0, 0, 0, 0)
@@ -189,9 +67,7 @@ class Folder extends Container, @folder {
  * The base name further decomposes into the _stem_ and _extension_ -- see
  * `getStem` and `getExtension`. To get the full path, use `getAbsolutePath`.
  */
-class File extends Container, @file {
-  override string getAbsolutePath() { files(underlyingElement(this), result) }
-
+class File extends Container, Impl::File {
   override string getAPrimaryQlClass() { result = "File" }
 
   override Location getLocation() {

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

@@ -79,13 +79,3 @@ class ArgumentPosition extends int {
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
-
-/**
- * Holds if flow from `call`'s argument `arg` to parameter `p` is permissible.
- *
- * This is a temporary hook to support technical debt in the Go language; do not use.
- */
-pragma[inline]
-predicate golangSpecificParamArgFilter(DataFlowCall call, ParameterNode p, ArgumentNode arg) {
-  any()
-}

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,12 +297,3 @@ class ContentApprox = Unit;
 /** Gets an approximated value for content `c`. */
 pragma[inline]
 ContentApprox getContentApprox(Content c) { any() }
-
-/**
- * Gets an additional term that is added to the `join` and `branch` computations to reflect
- * an additional forward or backwards branching factor that is not taken into account
- * when calculating the (virtual) dispatch cost.
- *
- * Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
- */
-int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) { none() }

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

@@ -271,13 +271,3 @@ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
-
-/**
- * Holds if flow from `call`'s argument `arg` to parameter `p` is permissible.
- *
- * This is a temporary hook to support technical debt in the Go language; do not use.
- */
-pragma[inline]
-predicate golangSpecificParamArgFilter(DataFlowCall call, ParameterNode p, ArgumentNode arg) {
-  any()
-}

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -297,6 +297,10 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
+  predicate isBarrierIn(Node node, FlowState state) { none() }
+
+  predicate isBarrierOut(Node node, FlowState state) { none() }
+
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

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

@@ -18,4 +18,6 @@ module CppDataFlow implements InputSig {
   import Public
 
   Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
+
+  predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
 }

cpp/ql/src/Critical/DoubleFree.ql

@@ -2,7 +2,7 @@
  * @name Potential double free
  * @description Freeing a resource more than once can lead to undefined behavior and cause memory corruption.
  * @kind path-problem
- * @precision medium
+ * @precision high
  * @id cpp/double-free
  * @problem.severity warning
  * @security-severity 9.3

cpp/ql/src/Critical/UseAfterFree.ql

@@ -2,7 +2,7 @@
  * @name Potential use after free
  * @description An allocated memory block is used after it has been freed. Behavior in such cases is undefined and can cause memory corruption.
  * @kind path-problem
- * @precision medium
+ * @precision high
  * @id cpp/use-after-free
  * @problem.severity warning
  * @security-severity 9.3
@@ -29,8 +29,7 @@ private predicate externalCallNeverDereferences(FormattingFunctionCall call, int
   )
 }
 
-predicate isUse0(DataFlow::Node n, Expr e) {
-  e = n.asExpr() and
+predicate isUse0(Expr e) {
   not isFree(_, e, _) and
   (
     e = any(PointerDereferenceExpr pde).getOperand()
@@ -43,7 +42,7 @@ predicate isUse0(DataFlow::Node n, Expr e) {
     or
     // Assume any function without a body will dereference the pointer
     exists(int i, Call call, Function f |
-      n.asExpr() = call.getArgument(i) and
+      e = call.getArgument(i) and
       f = call.getTarget() and
       not f.hasEntryPoint() and
       // Exclude known functions we know won't dereference the pointer.
@@ -57,7 +56,7 @@ module ParameterSinks {
   import semmle.code.cpp.ir.ValueNumbering
 
   predicate flowsToUse(DataFlow::Node n) {
-    isUse0(n, _)
+    isUse0(n.asExpr())
     or
     exists(DataFlow::Node succ |
       flowsToUse(succ) and
@@ -90,7 +89,7 @@ module ParameterSinks {
   ) {
     pragma[only_bind_out](source.asParameter()) = pragma[only_bind_out](init.getParameter()) and
     paramToUse(source, sink) and
-    isUse0(sink, _)
+    isUse0(sink.asExpr())
   }
 
   private InitializeParameterInstruction getAnAlwaysDereferencedParameter0() {
@@ -139,7 +138,7 @@ module IsUse {
   private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon
 
   predicate isUse(DataFlow::Node n, Expr e) {
-    isUse0(n, e)
+    isUse0(e) and n.asExpr() = e
     or
     exists(CallInstruction call, InitializeParameterInstruction init |
       n.asOperand().getDef().getUnconvertedResultExpression() = e and

cpp/ql/src/change-notes/2023-10-03-double-free.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* The `cpp/double-free` query has been further improved to reduce false positives and its precision has been increased from `medium` to `high`.

cpp/ql/src/change-notes/2023-10-03-use-after-free.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* The `cpp/use-after-free` query has been further improved to reduce false positives and its precision has been increased from `medium` to `high`.

cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected

@@ -96,6 +96,7 @@
 | test_free.cpp:255:10:255:10 | p |
 | test_free.cpp:260:9:260:9 | p |
 | test_free.cpp:263:12:263:12 | p |
+| test_free.cpp:269:7:269:11 | ... = ... |
 | virtual.cpp:18:10:18:10 | a |
 | virtual.cpp:19:10:19:10 | c |
 | virtual.cpp:38:10:38:10 | b |

cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryMayNotBeFreed.expected

@@ -1 +1,2 @@
 | test_free.cpp:36:22:36:35 | ... = ... | This memory allocation may not be released at $@. | test_free.cpp:38:1:38:1 | return ... | this exit point |
+| test_free.cpp:267:12:267:17 | call to malloc | This memory allocation may not be released at $@. | test_free.cpp:270:1:270:1 | return ... | this exit point |

cpp/ql/test/query-tests/Critical/MemoryFreed/test_free.cpp

@@ -261,4 +261,10 @@ void test_ref_delete(int *&p) {
 	p = new int;
 	use(p);  // GOOD
     delete p;  // GOOD
+}
+
+void test_free_assign() {
+	void *a = malloc(10); 
+	void *b;
+	free(b = a); // GOOD 
 }