Merge pull request #18226 from geoffw0/badcrypto

Rust: Weak encryption algorithm query.

Author: geoffw0

config/identical-files.json

@@ -234,12 +234,14 @@
   "CryptoAlgorithms Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
     "python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll",
-    "ruby/ql/lib/codeql/ruby/security/CryptoAlgorithms.qll"
+    "ruby/ql/lib/codeql/ruby/security/CryptoAlgorithms.qll",
+    "rust/ql/lib/codeql/rust/security/CryptoAlgorithms.qll"
   ],
   "CryptoAlgorithmNames Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/internal/CryptoAlgorithmNames.qll",
     "python/ql/lib/semmle/python/concepts/internal/CryptoAlgorithmNames.qll",
-    "ruby/ql/lib/codeql/ruby/security/internal/CryptoAlgorithmNames.qll"
+    "ruby/ql/lib/codeql/ruby/security/internal/CryptoAlgorithmNames.qll",
+    "rust/ql/lib/codeql/rust/security/internal/CryptoAlgorithmNames.qll"
   ],
   "SensitiveDataHeuristics Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
@@ -254,7 +256,8 @@
   "Concepts Python/Ruby/JS": [
     "python/ql/lib/semmle/python/internal/ConceptsShared.qll",
     "ruby/ql/lib/codeql/ruby/internal/ConceptsShared.qll",
-    "javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll"
+    "javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll",
+    "rust/ql/lib/codeql/rust/internal/ConceptsShared.qll"
   ],
   "ApiGraphModels": [
     "javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll",

javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll

@@ -30,7 +30,7 @@ module Cryptography {
   class PasswordHashingAlgorithm = CryptoAlgorithms::PasswordHashingAlgorithm;
 
   /**
-   * A data-flow node that is an application of a cryptographic algorithm. For example,
+   * A data flow node that is an application of a cryptographic algorithm. For example,
    * encryption, decryption, signature-validation.
    *
    * Extend this class to refine existing API models. If you want to model new APIs,
@@ -40,7 +40,7 @@ module Cryptography {
     /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
     CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
 
-    /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+    /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
     DataFlow::Node getInitialization() { result = super.getInitialization() }
 
     /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
@@ -61,14 +61,14 @@ module Cryptography {
   /** Provides classes for modeling new applications of a cryptographic algorithms. */
   module CryptographicOperation {
     /**
-     * A data-flow node that is an application of a cryptographic algorithm. For example,
+     * A data flow node that is an application of a cryptographic algorithm. For example,
      * encryption, decryption, signature-validation.
      *
      * Extend this class to model new APIs. If you want to refine existing API models,
      * extend `CryptographicOperation` instead.
      */
     abstract class Range extends DataFlow::Node {
-      /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+      /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
       abstract DataFlow::Node getInitialization();
 
       /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
@@ -118,14 +118,14 @@ module Http {
   /** Provides classes for modeling HTTP clients. */
   module Client {
     /**
-     * A data-flow node that makes an outgoing HTTP request.
+     * A data flow node that makes an outgoing HTTP request.
      *
      * Extend this class to refine existing API models. If you want to model new APIs,
      * extend `Http::Client::Request::Range` instead.
      */
     class Request extends DataFlow::Node instanceof Request::Range {
       /**
-       * Gets a data-flow node that contributes to the URL of the request.
+       * Gets a data flow node that contributes to the URL of the request.
        * Depending on the framework, a request may have multiple nodes which contribute to the URL.
        */
       DataFlow::Node getAUrlPart() { result = super.getAUrlPart() }
@@ -150,14 +150,14 @@ module Http {
     /** Provides a class for modeling new HTTP requests. */
     module Request {
       /**
-       * A data-flow node that makes an outgoing HTTP request.
+       * A data flow node that makes an outgoing HTTP request.
        *
        * Extend this class to model new APIs. If you want to refine existing API models,
        * extend `Http::Client::Request` instead.
        */
       abstract class Range extends DataFlow::Node {
         /**
-         * Gets a data-flow node that contributes to the URL of the request.
+         * Gets a data flow node that contributes to the URL of the request.
          * Depending on the framework, a request may have multiple nodes which contribute to the URL.
          */
         abstract DataFlow::Node getAUrlPart();

python/ql/lib/semmle/python/internal/ConceptsShared.qll

@@ -30,7 +30,7 @@ module Cryptography {
   class PasswordHashingAlgorithm = CryptoAlgorithms::PasswordHashingAlgorithm;
 
   /**
-   * A data-flow node that is an application of a cryptographic algorithm. For example,
+   * A data flow node that is an application of a cryptographic algorithm. For example,
    * encryption, decryption, signature-validation.
    *
    * Extend this class to refine existing API models. If you want to model new APIs,
@@ -40,7 +40,7 @@ module Cryptography {
     /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
     CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
 
-    /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+    /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
     DataFlow::Node getInitialization() { result = super.getInitialization() }
 
     /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
@@ -61,14 +61,14 @@ module Cryptography {
   /** Provides classes for modeling new applications of a cryptographic algorithms. */
   module CryptographicOperation {
     /**
-     * A data-flow node that is an application of a cryptographic algorithm. For example,
+     * A data flow node that is an application of a cryptographic algorithm. For example,
      * encryption, decryption, signature-validation.
      *
      * Extend this class to model new APIs. If you want to refine existing API models,
      * extend `CryptographicOperation` instead.
      */
     abstract class Range extends DataFlow::Node {
-      /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+      /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
       abstract DataFlow::Node getInitialization();
 
       /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
@@ -118,14 +118,14 @@ module Http {
   /** Provides classes for modeling HTTP clients. */
   module Client {
     /**
-     * A data-flow node that makes an outgoing HTTP request.
+     * A data flow node that makes an outgoing HTTP request.
      *
      * Extend this class to refine existing API models. If you want to model new APIs,
      * extend `Http::Client::Request::Range` instead.
      */
     class Request extends DataFlow::Node instanceof Request::Range {
       /**
-       * Gets a data-flow node that contributes to the URL of the request.
+       * Gets a data flow node that contributes to the URL of the request.
        * Depending on the framework, a request may have multiple nodes which contribute to the URL.
        */
       DataFlow::Node getAUrlPart() { result = super.getAUrlPart() }
@@ -150,14 +150,14 @@ module Http {
     /** Provides a class for modeling new HTTP requests. */
     module Request {
       /**
-       * A data-flow node that makes an outgoing HTTP request.
+       * A data flow node that makes an outgoing HTTP request.
        *
        * Extend this class to model new APIs. If you want to refine existing API models,
        * extend `Http::Client::Request` instead.
        */
       abstract class Range extends DataFlow::Node {
         /**
-         * Gets a data-flow node that contributes to the URL of the request.
+         * Gets a data flow node that contributes to the URL of the request.
          * Depending on the framework, a request may have multiple nodes which contribute to the URL.
          */
         abstract DataFlow::Node getAUrlPart();

ruby/ql/lib/codeql/ruby/internal/ConceptsShared.qll

@@ -30,7 +30,7 @@ module Cryptography {
   class PasswordHashingAlgorithm = CryptoAlgorithms::PasswordHashingAlgorithm;
 
   /**
-   * A data-flow node that is an application of a cryptographic algorithm. For example,
+   * A data flow node that is an application of a cryptographic algorithm. For example,
    * encryption, decryption, signature-validation.
    *
    * Extend this class to refine existing API models. If you want to model new APIs,
@@ -40,7 +40,7 @@ module Cryptography {
     /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
     CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
 
-    /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+    /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
     DataFlow::Node getInitialization() { result = super.getInitialization() }
 
     /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
@@ -61,14 +61,14 @@ module Cryptography {
   /** Provides classes for modeling new applications of a cryptographic algorithms. */
   module CryptographicOperation {
     /**
-     * A data-flow node that is an application of a cryptographic algorithm. For example,
+     * A data flow node that is an application of a cryptographic algorithm. For example,
      * encryption, decryption, signature-validation.
      *
      * Extend this class to model new APIs. If you want to refine existing API models,
      * extend `CryptographicOperation` instead.
      */
     abstract class Range extends DataFlow::Node {
-      /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+      /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
       abstract DataFlow::Node getInitialization();
 
       /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
@@ -118,14 +118,14 @@ module Http {
   /** Provides classes for modeling HTTP clients. */
   module Client {
     /**
-     * A data-flow node that makes an outgoing HTTP request.
+     * A data flow node that makes an outgoing HTTP request.
      *
      * Extend this class to refine existing API models. If you want to model new APIs,
      * extend `Http::Client::Request::Range` instead.
      */
     class Request extends DataFlow::Node instanceof Request::Range {
       /**
-       * Gets a data-flow node that contributes to the URL of the request.
+       * Gets a data flow node that contributes to the URL of the request.
        * Depending on the framework, a request may have multiple nodes which contribute to the URL.
        */
       DataFlow::Node getAUrlPart() { result = super.getAUrlPart() }
@@ -150,14 +150,14 @@ module Http {
     /** Provides a class for modeling new HTTP requests. */
     module Request {
       /**
-       * A data-flow node that makes an outgoing HTTP request.
+       * A data flow node that makes an outgoing HTTP request.
        *
        * Extend this class to model new APIs. If you want to refine existing API models,
        * extend `Http::Client::Request` instead.
        */
       abstract class Range extends DataFlow::Node {
         /**
-         * Gets a data-flow node that contributes to the URL of the request.
+         * Gets a data flow node that contributes to the URL of the request.
          * Depending on the framework, a request may have multiple nodes which contribute to the URL.
          */
         abstract DataFlow::Node getAUrlPart();

rust/ql/lib/codeql/rust/Concepts.qll

@@ -105,7 +105,7 @@ module RemoteSource {
 }
 
 /**
- * A data-flow node that constructs a SQL statement (for later execution).
+ * A data flow node that constructs a SQL statement (for later execution).
  *
  * Often, it is worthy of an alert if a SQL statement is constructed such that
  * executing it would be a security risk.
@@ -122,7 +122,7 @@ final class SqlConstruction = SqlConstruction::Range;
  */
 module SqlConstruction {
   /**
-   * A data-flow node that constructs a SQL statement.
+   * A data flow node that constructs a SQL statement.
    */
   abstract class Range extends DataFlow::Node {
     /**
@@ -133,7 +133,7 @@ module SqlConstruction {
 }
 
 /**
- * A data-flow node that constructs and executes SQL statements.
+ * A data flow node that constructs and executes SQL statements.
  *
  * If the context of interest is such that merely constructing a SQL statement
  * would be valuable to report, consider also using `SqlConstruction`.
@@ -148,7 +148,7 @@ final class SqlExecution = SqlExecution::Range;
  */
 module SqlExecution {
   /**
-   * A data-flow node that executes SQL statements.
+   * A data flow node that executes SQL statements.
    */
   abstract class Range extends DataFlow::Node {
     /**
@@ -159,7 +159,7 @@ module SqlExecution {
 }
 
 /**
- * A data-flow node that performs SQL sanitization.
+ * A data flow node that performs SQL sanitization.
  */
 final class SqlSanitization = SqlSanitization::Range;
 
@@ -168,7 +168,28 @@ final class SqlSanitization = SqlSanitization::Range;
  */
 module SqlSanitization {
   /**
-   * A data-flow node that performs SQL sanitization.
+   * A data flow node that performs SQL sanitization.
    */
   abstract class Range extends DataFlow::Node { }
 }
+
+/**
+ * Provides models for cryptographic things.
+ */
+module Cryptography {
+  private import codeql.rust.internal.ConceptsShared::Cryptography as SC
+
+  final class CryptographicOperation = SC::CryptographicOperation;
+
+  class EncryptionAlgorithm = SC::EncryptionAlgorithm;
+
+  class HashingAlgorithm = SC::HashingAlgorithm;
+
+  class PasswordHashingAlgorithm = SC::PasswordHashingAlgorithm;
+
+  module CryptographicOperation = SC::CryptographicOperation;
+
+  class BlockMode = SC::BlockMode;
+
+  class CryptographicAlgorithm = SC::CryptographicAlgorithm;
+}

rust/ql/lib/codeql/rust/Frameworks.qll

@@ -3,5 +3,6 @@
  */
 
 private import codeql.rust.frameworks.Reqwest
+private import codeql.rust.frameworks.RustCrypto
 private import codeql.rust.frameworks.stdlib.Env
 private import codeql.rust.frameworks.Sqlx

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -330,7 +330,7 @@ module Node {
     override ReturnKind getKind() { result = rk }
   }
 
-  /** A data-flow node that represents the output of a call. */
+  /** A data flow node that represents the output of a call. */
   abstract class OutNode extends Node {
     /** Gets the underlying call for this node. */
     abstract DataFlowCall getCall(ReturnKind kind);

rust/ql/lib/codeql/rust/frameworks/RustCrypto.qll

@@ -0,0 +1,57 @@
+/**
+ * Provides modeling for the `RustCrypto` family of crates (`cipher`, `digest` etc).
+ */
+
+private import rust
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.DataFlow
+
+bindingset[algorithmName]
+private string simplifyAlgorithmName(string algorithmName) {
+  // the cipher library gives triple-DES names like "TdesEee2" and "TdesEde2"
+  if algorithmName.matches("Tdes%") then result = "3des" else result = algorithmName
+}
+
+/**
+ * An operation that initializes a cipher through the `cipher::KeyInit` or
+ * `cipher::KeyIvInit` trait, for example `Des::new` or `cbc::Encryptor<des::Des>::new`.
+ */
+class StreamCipherInit extends Cryptography::CryptographicOperation::Range {
+  string algorithmName;
+
+  StreamCipherInit() {
+    // a call to `cipher::KeyInit::new`, `cipher::KeyInit::new_from_slice`,
+    // `cipher::KeyIvInit::new`, `cipher::KeyIvInit::new_from_slices` or `rc2::Rc2::new_with_eff_key_len`.
+    exists(PathExpr p, string rawAlgorithmName |
+      this.asExpr().getExpr().(CallExpr).getFunction() = p and
+      p.getResolvedCrateOrigin().matches("%/RustCrypto%") and
+      p.getPath().getPart().getNameRef().getText() =
+        ["new", "new_from_slice", "new_from_slices", "new_with_eff_key_len"] and
+      (
+        rawAlgorithmName = p.getPath().getQualifier().getPart().getNameRef().getText() or
+        rawAlgorithmName =
+          p.getPath()
+              .getQualifier()
+              .getPart()
+              .getGenericArgList()
+              .getGenericArg(0)
+              .(TypeArg)
+              .getTypeRepr()
+              .(PathTypeRepr)
+              .getPath()
+              .getPart()
+              .getNameRef()
+              .getText()
+      ) and
+      algorithmName = simplifyAlgorithmName(rawAlgorithmName)
+    )
+  }
+
+  override DataFlow::Node getInitialization() { result = this }
+
+  override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
+
+  override DataFlow::Node getAnInput() { none() }
+
+  override Cryptography::BlockMode getBlockMode() { result = "" }
+}

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

@@ -0,0 +1,7 @@
+/**
+ * This file contains imports required for the Rust version of `ConceptsShared.qll`.
+ * Since they are language-specific, they can't be placed directly in that file, as it is shared between languages.
+ */
+
+import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
+import codeql.rust.security.CryptoAlgorithms as CryptoAlgorithms