Merge branch 'main' into post-release-prep/codeql-cli-2.22.2

Author: nickrolfe

rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected

@@ -15,6 +15,7 @@ ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
 ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
 ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
+ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
 ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
 ql/rust/ql/src/queries/summary/LinesOfCode.ql
 ql/rust/ql/src/queries/summary/LinesOfUserCode.ql

rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

@@ -16,6 +16,7 @@ ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
 ql/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
 ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
+ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
 ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
 ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
 ql/rust/ql/src/queries/summary/LinesOfCode.ql

rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

@@ -15,6 +15,7 @@ ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
 ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
 ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
+ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
 ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
 ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
 ql/rust/ql/src/queries/summary/LinesOfCode.ql

rust/ql/lib/codeql/rust/frameworks/genericarray.model.yml

@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: summaryModel
+    data:
+      - ["<generic_array::GenericArray>::from_slice", "Argument[0].Reference", "ReturnValue.Reference", "value", "manual"]
+      - ["<generic_array::GenericArray>::from_mut_slice", "Argument[0].Reference", "ReturnValue.Reference", "value", "manual"]
+      - ["<generic_array::GenericArray>::try_from_slice", "Argument[0].Reference", "ReturnValue.Field[crate::result::Result::Ok(0)].Reference", "value", "manual"]
+      - ["<generic_array::GenericArray>::try_from_mut_slice", "Argument[0].Reference", "ReturnValue.Field[crate::result::Result::Ok(0)].Reference", "value", "manual"]

rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml

@@ -8,3 +8,28 @@ extensions:
       - ["<_ as digest::digest::Digest>::chain_update", "Argument[0]", "hasher-input", "manual"]
       - ["<_ as digest::digest::Digest>::digest", "Argument[0]", "hasher-input", "manual"]
       - ["md5::compute", "Argument[0]", "hasher-input", "manual"]
+      - ["<_ as crypto_common::KeyInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<_ as crypto_common::KeyInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<_ as crypto_common::KeyInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<_ as crypto_common::KeyInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<_ as crypto_common::KeyIvInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<_ as crypto_common::KeyIvInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<_ as crypto_common::KeyIvInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<_ as crypto_common::KeyIvInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyIvInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyIvInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyIvInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<cipher::stream_wrapper::StreamCipherCoreWrapper as crypto_common::KeyIvInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyIvInit>::new", "Argument[0]", "credentials-key", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyIvInit>::new", "Argument[1]", "credentials-iv", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyIvInit>::new_from_slice", "Argument[0]", "credentials-key", "manual"]
+      - ["<digest::core_api::wrapper::CoreWrapper as crypto_common::KeyIvInit>::new_from_slice", "Argument[1]", "credentials-iv", "manual"]
+      - ["<_ as aead::Aead>::encrypt", "Argument[0]", "credentials-nonce", "manual"]

rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml

@@ -3,6 +3,12 @@ extensions:
       pack: codeql/rust-all
       extensible: summaryModel
     data:
+      # Conversions
+      - ["<core::alloc::layout::Layout>::align_to", "Argument[self].Element", "ReturnValue.Field[0,1,2].Reference.Element", "taint", "manual"]
+      - ["<_ as core::convert::Into>::into", "Argument[self].Element", "ReturnValue.Element", "taint", "manual"]
+      - ["<_ as core::convert::Into>::into", "Argument[self].Reference.Element", "ReturnValue.Element", "taint", "manual"]
+      - ["<alloc::string::String as core::convert::Into>::into", "Argument[self].Element", "ReturnValue.Element", "taint", "manual"]
+      - ["<alloc::string::String as core::convert::Into>::into", "Argument[self].Reference.Element", "ReturnValue.Element", "taint", "manual"]
       # Iterator
       - ["<core::result::Result>::iter", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
       - ["<alloc::vec::Vec as value_trait::array::Array>::iter", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
@@ -59,6 +65,8 @@ extensions:
       pack: codeql/rust-all
       extensible: sourceModel
     data:
+      # Mem
+      - ["core::mem::zeroed", "ReturnValue.Element", "constant-source", "manual"]
       # Ptr
       - ["core::ptr::drop_in_place", "Argument[0]", "pointer-invalidate", "manual"]
       - ["core::ptr::dangling", "ReturnValue", "pointer-invalidate", "manual"]

rust/ql/lib/codeql/rust/internal/PathResolution.qll

@@ -112,13 +112,18 @@ abstract class ItemNode extends Locatable {
     result = this.(SourceFileItemNode).getSuper()
   }
 
+  pragma[nomagic]
+  private ItemNode getAChildSuccessor(string name) {
+    this = result.getImmediateParent() and
+    name = result.getName()
+  }
+
   cached
   ItemNode getASuccessorRec(string name) {
     Stages::PathResolutionStage::ref() and
     sourceFileEdge(this, name, result)
     or
-    this = result.getImmediateParent() and
-    name = result.getName()
+    result = this.getAChildSuccessor(name)
     or
     fileImportEdge(this, name, result)
     or
@@ -224,6 +229,38 @@ abstract class ItemNode extends Locatable {
     result.(CrateItemNode).isPotentialDollarCrateTarget()
   }
 
+  /**
+   * Holds if the successor `item` with the name `name` is not available locally
+   * for unqualified paths.
+   *
+   * This has the effect that a path of the form `name` inside `this` will not
+   * resolve to `item`.
+   */
+  pragma[nomagic]
+  predicate excludedLocally(string name, ItemNode item) {
+    // Associated items in an impl or trait block are not directly available
+    // inside the block, they require a qualified path with a `Self` prefix.
+    item = this.getAChildSuccessor(name) and
+    this instanceof ImplOrTraitItemNode and
+    item instanceof AssocItemNode
+  }
+
+  /**
+   * Holds if the successor `item` with the name `name` is not available
+   * externally for qualified paths that resolve to this item.
+   *
+   * This has the effect that a path of the form `Qualifier::name`, where
+   * `Qualifier` resolves to this item, will not resolve to `item`.
+   */
+  pragma[nomagic]
+  predicate excludedExternally(string name, ItemNode item) {
+    // Type parameters for an `impl` or trait block are not available outside of
+    // the block.
+    item = this.getAChildSuccessor(name) and
+    this instanceof ImplOrTraitItemNode and
+    item instanceof TypeParamItemNode
+  }
+
   pragma[nomagic]
   private predicate hasSourceFunction(string name) {
     this.getASuccessorFull(name).(Function).fromSource()
@@ -1145,7 +1182,9 @@ pragma[nomagic]
 private predicate declares(ItemNode item, Namespace ns, string name) {
   exists(ItemNode child | child.getImmediateParent() = item |
     child.getName() = name and
-    child.getNamespace() = ns
+    child.getNamespace() = ns and
+    // If `item` is excluded locally then it does not declare `name`.
+    not item.excludedLocally(name, child)
     or
     useTreeDeclares(child.(Use).getUseTree(), name) and
     exists(ns) // `use foo::bar` can refer to both a value and a type
@@ -1193,38 +1232,27 @@ private ItemNode getOuterScope(ItemNode i) {
   result = i.getImmediateParent()
 }
 
-pragma[nomagic]
-private ItemNode getAdjustedEnclosing(ItemNode encl0, Namespace ns) {
-  // functions in `impl` blocks need to use explicit `Self::` to access other
-  // functions in the `impl` block
-  if encl0 instanceof ImplOrTraitItemNode and ns.isValue()
-  then result = encl0.getImmediateParent()
-  else result = encl0
-}
-
 /**
  * Holds if the unqualified path `p` references an item named `name`, and `name`
  * may be looked up in the `ns` namespace inside enclosing item `encl`.
  */
 pragma[nomagic]
 private predicate unqualifiedPathLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
-  exists(ItemNode encl0 | encl = getAdjustedEnclosing(encl0, ns) |
-    // lookup in the immediately enclosing item
-    p.isUnqualified(name) and
-    encl0.getADescendant() = p and
-    exists(ns) and
-    not name = ["crate", "$crate", "super", "self"]
-    or
-    // lookup in an outer scope, but only if the item is not declared in inner scope
-    exists(ItemNode mid |
-      unqualifiedPathLookup(mid, name, ns, p) and
-      not declares(mid, ns, name) and
-      not (
-        name = "Self" and
-        mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
-      ) and
-      encl0 = getOuterScope(mid)
-    )
+  // lookup in the immediately enclosing item
+  p.isUnqualified(name) and
+  encl.getADescendant() = p and
+  exists(ns) and
+  not name = ["crate", "$crate", "super", "self"]
+  or
+  // lookup in an outer scope, but only if the item is not declared in inner scope
+  exists(ItemNode mid |
+    unqualifiedPathLookup(mid, name, ns, p) and
+    not declares(mid, ns, name) and
+    not (
+      name = "Self" and
+      mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
+    ) and
+    encl = getOuterScope(mid)
   )
 }
 
@@ -1245,10 +1273,10 @@ private predicate sourceFileHasCratePathTc(ItemNode i1, ItemNode i2) =
 
 /**
  * Holds if the unqualified path `p` references a keyword item named `name`, and
- * `name` may be looked up in the `ns` namespace inside enclosing item `encl`.
+ * `name` may be looked up inside enclosing item `encl`.
  */
 pragma[nomagic]
-private predicate keywordLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
+private predicate keywordLookup(ItemNode encl, string name, RelevantPath p) {
   // For `($)crate`, jump directly to the root module
   exists(ItemNode i | p.isCratePath(name, i) |
     encl instanceof SourceFile and
@@ -1259,18 +1287,17 @@ private predicate keywordLookup(ItemNode encl, string name, Namespace ns, Releva
   or
   name = ["super", "self"] and
   p.isUnqualified(name) and
-  exists(ItemNode encl0 |
-    encl0.getADescendant() = p and
-    encl = getAdjustedEnclosing(encl0, ns)
-  )
+  encl.getADescendant() = p
 }
 
 pragma[nomagic]
 private ItemNode unqualifiedPathLookup(RelevantPath p, Namespace ns) {
-  exists(ItemNode encl, string name | result = getASuccessorFull(encl, name, ns) |
+  exists(ItemNode encl, string name |
+    result = getASuccessorFull(encl, name, ns) and not encl.excludedLocally(name, result)
+  |
     unqualifiedPathLookup(encl, name, ns, p)
     or
-    keywordLookup(encl, name, ns, p)
+    keywordLookup(encl, name, p) and exists(ns)
   )
 }
 
@@ -1291,7 +1318,8 @@ private ItemNode resolvePath0(RelevantPath path, Namespace ns) {
   or
   exists(ItemNode q, string name |
     q = resolvePathQualifier(path, name) and
-    result = getASuccessorFull(q, name, ns)
+    result = getASuccessorFull(q, name, ns) and
+    not q.excludedExternally(name, result)
   )
   or
   result = resolveUseTreeListItem(_, _, path) and

rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll

@@ -5,7 +5,7 @@
 
 import rust
 private import codeql.rust.dataflow.DataFlow
-private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.dataflow.FlowSink
 private import codeql.rust.security.SensitiveData
 private import codeql.rust.Concepts
 

rust/ql/lib/codeql/rust/security/HardcodedCryptographicValueExtensions.qll

@@ -0,0 +1,109 @@
+/**
+ * Provides classes and predicates for reasoning about hard-coded cryptographic value
+ * vulnerabilities.
+ */
+
+import rust
+private import codeql.rust.dataflow.DataFlow
+private import codeql.rust.dataflow.FlowSource
+private import codeql.rust.dataflow.FlowSink
+private import codeql.rust.Concepts
+private import codeql.rust.security.SensitiveData
+
+/**
+ * A kind of cryptographic value.
+ */
+class CryptographicValueKind extends string {
+  CryptographicValueKind() { this = ["password", "key", "iv", "nonce", "salt"] }
+
+  /**
+   * Gets a description of this value kind for user-facing messages.
+   */
+  string getDescription() {
+    this = "password" and result = "a password"
+    or
+    this = "key" and result = "a key"
+    or
+    this = "iv" and result = "an initialization vector"
+    or
+    this = "nonce" and result = "a nonce"
+    or
+    this = "salt" and result = "a salt"
+  }
+}
+
+/**
+ * Provides default sources, sinks and barriers for detecting hard-coded cryptographic
+ * value vulnerabilities, as well as extension points for adding your own.
+ */
+module HardcodedCryptographicValue {
+  /**
+   * A data flow source for hard-coded cryptographic value vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for hard-coded cryptographic value vulnerabilities.
+   */
+  abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "HardcodedCryptographicValue" }
+
+    /**
+     * Gets the kind of credential this sink is interpreted as.
+     */
+    abstract CryptographicValueKind getKind();
+  }
+
+  /**
+   * A barrier for hard-coded cryptographic value vulnerabilities.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * A literal, considered as a flow source.
+   */
+  private class LiteralSource extends Source {
+    LiteralSource() { this.asExpr().getExpr() instanceof LiteralExpr }
+  }
+
+  /**
+   * An array initialized from a list of literals, considered as a single flow source. For example:
+   * ```
+   * `[0, 0, 0, 0]`
+   * ```
+   */
+  private class ArrayListSource extends Source {
+    ArrayListSource() { this.asExpr().getExpr().(ArrayListExpr).getExpr(_) instanceof LiteralExpr }
+  }
+
+  /**
+   * An externally modeled source for constant values.
+   */
+  private class ModeledSource extends Source {
+    ModeledSource() { sourceNode(this, "constant-source") }
+  }
+
+  /**
+   * An externally modeled sink for hard-coded cryptographic value vulnerabilities.
+   */
+  private class ModelsAsDataSinks extends Sink {
+    CryptographicValueKind kind;
+
+    ModelsAsDataSinks() { sinkNode(this, "credentials-" + kind) }
+
+    override CryptographicValueKind getKind() { result = kind }
+  }
+
+  /**
+   * A call to `getrandom` that is a barrier.
+   */
+  private class GetRandomBarrier extends Barrier {
+    GetRandomBarrier() {
+      exists(CallExprBase ce |
+        ce.getStaticTarget().(Addressable).getCanonicalPath() =
+          ["getrandom::fill", "getrandom::getrandom"] and
+        this.asExpr().getExpr().getParentNode*() = ce.getArgList().getArg(0)
+      )
+    }
+  }
+}

rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll

@@ -6,7 +6,7 @@
 
 import rust
 private import codeql.rust.dataflow.DataFlow
-private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.dataflow.FlowSink
 private import codeql.rust.Concepts
 private import codeql.util.Unit