Python: Port py/weak-crypto-key to use type-tracking

docs/codeql/support/reusables/frameworks.rst

@@ -147,3 +147,6 @@ Python built-in support
    MySQLdb, Database
    psycopg2, Database
    sqlite3, Database
+   cryptography, Cryptography library
+   pycryptodome, Cryptography library
+   pycryptodomex, Cryptography library

python/change-notes/2021-02-02-port-weak-crypto-key-query.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Updated _Use of weak cryptographic key_ (`py/weak-crypto-key`) query to use the new type-tracking approach instead of points-to analysis. You may see differences in the results found by the query, but overall this change should result in a more robust and accurate analysis.
+* Renamed the query file for _Use of weak cryptographic key_ (`py/weak-crypto-key`) from `WeakCrypto.ql` to `WeakCryptoKey.ql` (in the `python/ql/src/Security/CWE-326/` folder). This will affect any custom query suites that include or exclude this query using its path.

python/ql/src/Security/CWE-326/WeakCryptoKey.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Use of weak cryptographic key
+ * @description Use of a cryptographic key that is too small may allow the encryption to be broken.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id py/weak-crypto-key
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import python
+import semmle.python.Concepts
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.filters.Tests
+
+from Cryptography::PublicKey::KeyGeneration keyGen, int keySize, DataFlow::Node origin
+where
+  keySize = keyGen.getKeySizeWithOrigin(origin) and
+  keySize < keyGen.minimumSecureKeySize() and
+  not origin.getScope().getScope*() instanceof TestScope
+select keyGen,
+  "Creation of an " + keyGen.getName() + " key uses $@ bits, which is below " +
+    keyGen.minimumSecureKeySize() + " and considered breakable.", origin, keySize.toString()

python/ql/src/semmle/python/Concepts.qll

@@ -526,3 +526,116 @@ module HTTP {
     }
   }
 }
+
+/** Provides models for cryptographic things. */
+module Cryptography {
+  /** Provides models for public-key cryptography, also called asymmetric cryptography. */
+  module PublicKey {
+    /**
+     * A data-flow node that generates a new key-pair for use with public-key cryptography.
+     *
+     * Extend this class to refine existing API models. If you want to model new APIs,
+     * extend `KeyGeneration::Range` instead.
+     */
+    class KeyGeneration extends DataFlow::Node {
+      KeyGeneration::Range range;
+
+      KeyGeneration() { this = range }
+
+      /** Gets the name of the cryptographic algorithm (for example `"RSA"` or `"AES"`). */
+      string getName() { result = range.getName() }
+
+      /** Gets the argument that specifies the size of the key in bits, if available. */
+      DataFlow::Node getKeySizeArg() { result = range.getKeySizeArg() }
+
+      /**
+       * Gets the size of the key generated (in bits), as well as the `origin` that
+       * explains how we obtained this specific key size.
+       */
+      int getKeySizeWithOrigin(DataFlow::Node origin) {
+        result = range.getKeySizeWithOrigin(origin)
+      }
+
+      /** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
+      int minimumSecureKeySize() { result = range.minimumSecureKeySize() }
+    }
+
+    /** Provides classes for modeling new key-pair generation APIs. */
+    module KeyGeneration {
+      /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
+      DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::TypeBackTracker t, DataFlow::Node arg) {
+        t.start() and
+        arg = any(KeyGeneration::Range r).getKeySizeArg() and
+        result = arg.getALocalSource()
+        or
+        // Due to bad performance when using normal setup with we have inlined that code and forced a join
+        exists(DataFlow::TypeBackTracker t2 |
+          exists(DataFlow::StepSummary summary |
+            keysizeBacktracker_first_join(t2, arg, result, summary) and
+            t = t2.prepend(summary)
+          )
+        )
+      }
+
+      pragma[nomagic]
+      private predicate keysizeBacktracker_first_join(
+        DataFlow::TypeBackTracker t2, DataFlow::Node arg, DataFlow::Node res,
+        DataFlow::StepSummary summary
+      ) {
+        DataFlow::StepSummary::step(res, keysizeBacktracker(t2, arg), summary)
+      }
+
+      /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
+      DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::Node arg) {
+        result = keysizeBacktracker(DataFlow::TypeBackTracker::end(), arg)
+      }
+
+      /**
+       * A data-flow node that generates a new key-pair for use with public-key cryptography.
+       *
+       * Extend this class to model new APIs. If you want to refine existing API models,
+       * extend `KeyGeneration` instead.
+       */
+      abstract class Range extends DataFlow::Node {
+        /** Gets the name of the cryptographic algorithm (for example `"RSA"`). */
+        abstract string getName();
+
+        /** Gets the argument that specifies the size of the key in bits, if available. */
+        abstract DataFlow::Node getKeySizeArg();
+
+        /**
+         * Gets the size of the key generated (in bits), as well as the `origin` that
+         * explains how we obtained this specific key size.
+         */
+        int getKeySizeWithOrigin(DataFlow::Node origin) {
+          origin = keysizeBacktracker(this.getKeySizeArg()) and
+          result = origin.asExpr().(IntegerLiteral).getValue()
+        }
+
+        /** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
+        abstract int minimumSecureKeySize();
+      }
+
+      /** A data-flow node that generates a new RSA key-pair. */
+      abstract class RsaRange extends Range {
+        final override string getName() { result = "RSA" }
+
+        final override int minimumSecureKeySize() { result = 2048 }
+      }
+
+      /** A data-flow node that generates a new DSA key-pair. */
+      abstract class DsaRange extends Range {
+        final override string getName() { result = "DSA" }
+
+        final override int minimumSecureKeySize() { result = 2048 }
+      }
+
+      /** A data-flow node that generates a new ECC key-pair. */
+      abstract class EccRange extends Range {
+        final override string getName() { result = "ECC" }
+
+        final override int minimumSecureKeySize() { result = 224 }
+      }
+    }
+  }
+}

python/ql/src/semmle/python/Frameworks.qll

@@ -2,6 +2,8 @@
  * Helper file that imports all framework modeling.
  */
 
+private import semmle.python.frameworks.Cryptodome
+private import semmle.python.frameworks.Cryptography
 private import semmle.python.frameworks.Dill
 private import semmle.python.frameworks.Django
 private import semmle.python.frameworks.Fabric

python/ql/src/semmle/python/frameworks/Cryptodome.qll

@@ -0,0 +1,104 @@
+/**
+ * Provides classes modeling security-relevant aspects of
+ * - the `pycryptodome` PyPI package (imported as `Crypto`)
+ * - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
+ * See https://pycryptodome.readthedocs.io/en/latest/.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
+
+/**
+ * Provides models for
+ * - the `pycryptodome` PyPI package (imported as `Crypto`)
+ * - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
+ * See https://pycryptodome.readthedocs.io/en/latest/
+ */
+private module CryptodomeModel {
+  // ---------------------------------------------------------------------------
+  /**
+   * A call to `Cryptodome.PublicKey.RSA.generate`/`Crypto.PublicKey.RSA.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/rsa.html#Crypto.PublicKey.RSA.generate
+   */
+  class CryptodomePublicKeyRsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::RsaRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyRsaGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("RSA")
+            .getMember("generate")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(0), this.getArgByName("bits")]
+    }
+  }
+
+  /**
+   * A call to `Cryptodome.PublicKey.DSA.generate`/`Crypto.PublicKey.DSA.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/dsa.html#Crypto.PublicKey.DSA.generate
+   */
+  class CryptodomePublicKeyDsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::DsaRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyDsaGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("DSA")
+            .getMember("generate")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(0), this.getArgByName("bits")]
+    }
+  }
+
+  /**
+   * A call to `Cryptodome.PublicKey.ECC.generate`/`Crypto.PublicKey.ECC.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html#Crypto.PublicKey.ECC.generate
+   */
+  class CryptodomePublicKeyEccGenerateCall extends Cryptography::PublicKey::KeyGeneration::EccRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyEccGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("ECC")
+            .getMember("generate")
+            .getACall()
+    }
+
+    /** Gets the argument that specifies the curve to use (a string). */
+    DataFlow::Node getCurveArg() { result = this.getArgByName("curve") }
+
+    /** Gets the name of the curve to use, as well as the origin that explains how we obtained this name. */
+    string getCurveWithOrigin(DataFlow::Node origin) {
+      exists(StrConst str | origin = DataFlow::exprNode(str) |
+        origin = this.getCurveArg().getALocalSource() and
+        result = str.getText()
+      )
+    }
+
+    override int getKeySizeWithOrigin(DataFlow::Node origin) {
+      exists(string curve | curve = this.getCurveWithOrigin(origin) |
+        // using list from https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html
+        curve in ["NIST P-256", "p256", "P-256", "prime256v1", "secp256r1"] and result = 256
+        or
+        curve in ["NIST P-384", "p384", "P-384", "prime384v1", "secp384r1"] and result = 384
+        or
+        curve in ["NIST P-521", "p521", "P-521", "prime521v1", "secp521r1"] and result = 521
+      )
+    }
+
+    // Note: There is not really a key-size argument, since it's always specified by the curve.
+    override DataFlow::Node getKeySizeArg() { none() }
+  }
+}