Expand model and specialize newtype relations

Author: nicolaswill

cpp/ql/lib/experimental/Quantum/OpenSSL.qll

@@ -4,16 +4,6 @@ import semmle.code.cpp.dataflow.new.DataFlow
 module OpenSSLModel {
   import Language
 
-  class FunctionCallOrMacroAccess extends Element {
-    FunctionCallOrMacroAccess() { this instanceof FunctionCall or this instanceof MacroAccess }
-
-    string getTargetName() {
-      result = this.(FunctionCall).getTarget().getName()
-      or
-      result = this.(MacroAccess).getMacroName()
-    }
-  }
-
   /**
    * Hash function references in OpenSSL.
    */
@@ -42,13 +32,27 @@ module OpenSSLModel {
     hash_ref_type_mapping_known(name, algo)
   }
 
-  class HashAlgorithmRef extends Crypto::HashAlgorithm {
-    FunctionCallOrMacroAccess instance;
+  class FunctionCallOrMacroAccess extends Element {
+    FunctionCallOrMacroAccess() { this instanceof FunctionCall or this instanceof MacroAccess }
 
-    HashAlgorithmRef() {
-      this = Crypto::THashAlgorithm(instance) and
-      hash_ref_type_mapping(instance, _, _)
+    string getTargetName() {
+      result = this.(FunctionCall).getTarget().getName()
+      or
+      result = this.(MacroAccess).getMacroName()
     }
+  }
+
+  class HashAlgorithmCallOrMacro extends Crypto::HashAlgorithmInstance instanceof FunctionCallOrMacroAccess
+  {
+    HashAlgorithmCallOrMacro() { hash_ref_type_mapping(this, _, _) }
+
+    string getTargetName() { result = this.(FunctionCallOrMacroAccess).getTargetName() }
+  }
+
+  class HashAlgorithm extends Crypto::HashAlgorithm {
+    HashAlgorithmCallOrMacro instance;
+
+    HashAlgorithm() { this = Crypto::THashAlgorithm(instance) }
 
     override string getSHA2OrSHA3DigestSize(Location location) {
       (
@@ -81,9 +85,9 @@ module OpenSSLModel {
 
     predicate isSink(DataFlow::Node sink) {
       exists(EVP_KDF_derive kdo |
-        sink.asExpr() = kdo.getAlgorithmArg()
+        sink.asExpr() = kdo.getCall().getAlgorithmArg()
         or
-        sink.asExpr() = kdo.getContextArg() // via `EVP_KDF_CTX_set_params`
+        sink.asExpr() = kdo.getCall().getContextArg() // via `EVP_KDF_CTX_set_params`
       )
     }
 
@@ -101,21 +105,23 @@ module OpenSSLModel {
   /**
    * Key derivation operation (e.g., `EVP_KDF_derive`)
    */
-  abstract class KeyDerivationOperation extends Crypto::KeyDerivationOperation { }
+  class EVP_KDF_derive_FunctionCall extends Crypto::KeyDerivationOperationInstance instanceof FunctionCall
+  {
+    EVP_KDF_derive_FunctionCall() { this.getTarget().getName() = "EVP_KDF_derive" }
 
-  class EVP_KDF_derive extends KeyDerivationOperation {
-    FunctionCall instance;
+    Expr getAlgorithmArg() { result = super.getArgument(3) }
 
-    EVP_KDF_derive() {
-      this = Crypto::TKeyDerivationOperation(instance) and
-      instance.getTarget().getName() = "EVP_KDF_derive"
-    }
+    Expr getContextArg() { result = super.getArgument(0) }
+  }
 
-    override Crypto::Algorithm getAlgorithm() { algorithm_to_EVP_KDF_derive(result, this) }
+  class EVP_KDF_derive extends Crypto::KeyDerivationOperation {
+    EVP_KDF_derive_FunctionCall instance;
+
+    EVP_KDF_derive() { this = Crypto::TKeyDerivationOperation(instance) }
 
-    Expr getAlgorithmArg() { result = instance.getArgument(3) }
+    override Crypto::Algorithm getAlgorithm() { algorithm_to_EVP_KDF_derive(result, this) }
 
-    Expr getContextArg() { result = instance.getArgument(0) }
+    EVP_KDF_derive_FunctionCall getCall() { result = instance }
   }
 
   /**
@@ -134,7 +140,7 @@ module OpenSSLModel {
     Expr getAlgorithmArg() { result = this.getArgument(1) }
   }
 
-  class EVP_KDF_fetch_AlgorithmArg extends Expr {
+  class EVP_KDF_fetch_AlgorithmArg extends Crypto::KeyDerivationAlgorithmInstance instanceof Expr {
     EVP_KDF_fetch_AlgorithmArg() { exists(EVP_KDF_fetch_Call call | this = call.getAlgorithmArg()) }
   }
 

java/ql/lib/experimental/Quantum/JCA.qll

@@ -4,39 +4,39 @@ import semmle.code.java.dataflow.DataFlow
 module JCAModel {
   import Language
 
-  abstract class EncryptionOperation extends Crypto::EncryptionOperation { }
-
-  //TODO PBEWith can have suffixes. how to do? enumerate? or match a pattern?
+  // TODO: Verify that the PBEWith% case works correctly
   bindingset[algo]
   predicate cipher_names(string algo) {
-    // "Standard names are not case-sensitive."
     algo.toUpperCase()
         .matches([
             "AES", "AESWrap", "AESWrapPad", "ARCFOUR", "Blowfish", "ChaCha20", "ChaCha20-Poly1305",
             "DES", "DESede", "DESedeWrap", "ECIES", "PBEWith%", "RC2", "RC4", "RC5", "RSA"
           ].toUpperCase())
   }
 
-  //TODO solve the fact that x is an int of various values. same as above... enumerate?
+  // TODO: Verify that the CFB% case works correctly
+  bindingset[mode]
   predicate cipher_modes(string mode) {
-    mode =
-      [
-        "NONE", "CBC", "CCM", "CFB", "CFBx", "CTR", "CTS", "ECB", "GCM", "KW", "KWP", "OFB", "OFBx",
-        "PCBC"
-      ]
+    mode.toUpperCase()
+        .matches([
+            "NONE", "CBC", "CCM", "CFB", "CFB%", "CTR", "CTS", "ECB", "GCM", "KW", "KWP", "OFB",
+            "OFB%", "PCBC"
+          ].toUpperCase())
   }
 
-  //todo same as above, OAEPWith has asuffix type
+  // TODO: Verify that the OAEPWith% case works correctly
+  bindingset[padding]
   predicate cipher_padding(string padding) {
-    padding =
-      [
-        "NoPadding", "ISO10126Padding", "OAEPPadding", "OAEPWith", "PKCS1Padding", "PKCS5Padding",
-        "SSL3Padding"
-      ]
+    padding
+        .toUpperCase()
+        .matches([
+            "NoPadding", "ISO10126Padding", "OAEPPadding", "OAEPWith%", "PKCS1Padding",
+            "PKCS5Padding", "SSL3Padding"
+          ].toUpperCase())
   }
 
   /**
-   * this may be specified either in the ALG/MODE/PADDING or just ALG format
+   * A `StringLiteral` in the `"ALG/MODE/PADDING"` or `"ALG"` format
    */
   class CipherStringLiteral extends StringLiteral {
     CipherStringLiteral() { cipher_names(this.getValue().splitAt("/")) }
@@ -56,6 +56,9 @@ module JCAModel {
     Expr getAlgorithmArg() { result = this.getArgument(0) }
   }
 
+  /**
+   * Data-flow configuration modelling flow from a cipher string literal to a `CipherGetInstanceCall` argument.
+   */
   private module AlgorithmStringToFetchConfig implements DataFlow::ConfigSig {
     predicate isSource(DataFlow::Node src) { src.asExpr() instanceof CipherStringLiteral }
 
@@ -66,70 +69,77 @@ module JCAModel {
 
   module AlgorithmStringToFetchFlow = DataFlow::Global<AlgorithmStringToFetchConfig>;
 
-  class CipherGetInstanceAlgorithmArg extends Expr {
+  /**
+   * The cipher algorithm argument to a `CipherGetInstanceCall`.
+   *
+   * For example, in `Cipher.getInstance(algorithm)`, this class represents `algorithm`.
+   */
+  class CipherGetInstanceAlgorithmArg extends Crypto::EncryptionAlgorithmInstance,
+    Crypto::ModeOfOperationAlgorithmInstance instanceof Expr
+  {
     CipherGetInstanceAlgorithmArg() {
       exists(CipherGetInstanceCall call | this = call.getArgument(0))
     }
 
-    StringLiteral getOrigin() {
-      AlgorithmStringToFetchFlow::flow(DataFlow::exprNode(result), DataFlow::exprNode(this))
+    /**
+     * Returns the `StringLiteral` from which this argument is derived, if known.
+     */
+    CipherStringLiteral getOrigin() {
+      AlgorithmStringToFetchFlow::flow(DataFlow::exprNode(result),
+        DataFlow::exprNode(this.(Expr).getAChildExpr*()))
     }
   }
 
-  class ModeStringLiteral extends Crypto::ModeOfOperation {
-    CipherStringLiteral instance;
+  /**
+   * A block cipher mode of operation, where the mode is specified in the ALG or ALG/MODE/PADDING format.
+   *
+   * This class will only exist when the mode (*and its type*) is determinable.
+   * This is because the mode will always be specified alongside the algorithm and never independently.
+   * Therefore, we can always assume that a determinable algorithm will have a determinable mode.
+   *
+   * In the case that only an algorithm is specified, e.g., "AES", the provider provides a default mode.
+   *
+   * TODO: Model the case of relying on a provider default, but alert on it as a bad practice.
+   */
+  class ModeOfOperation extends Crypto::ModeOfOperationAlgorithm {
+    CipherGetInstanceAlgorithmArg instance;
 
-    ModeStringLiteral() {
+    ModeOfOperation() {
       this = Crypto::TModeOfOperationAlgorithm(instance) and
-      exists(instance.getMode()) and
-      instance = any(CipherGetInstanceAlgorithmArg call).getOrigin()
+      // TODO: this currently only holds for explicitly defined modes in a string literal.
+      // Cases with defaults, e.g., "AES", are not yet modelled.
+      // For these cases, in a CBOM, the AES node would have an unknown edge to its mode child.
+      exists(instance.getOrigin().getMode())
     }
 
     override Location getLocation() { result = instance.getLocation() }
 
-    override string getRawAlgorithmName() { result = instance.getMode() }
+    override string getRawAlgorithmName() { result = instance.getOrigin().getValue() }
 
     predicate modeToNameMapping(Crypto::TModeOperationType type, string name) {
       super.modeToNameMapping(type, name)
     }
 
     override Crypto::TModeOperationType getModeType() {
-      this.modeToNameMapping(result, instance.getMode().toUpperCase())
+      this.modeToNameMapping(result, instance.getOrigin().getMode())
     }
 
     CipherStringLiteral getInstance() { result = instance }
   }
 
-  //todo refactor
-  // class CipherAlgorithmPaddingStringLiteral extends CipherAlgorithmPadding instanceof StringLiteral {
-  //   CipherAlgorithmPaddingStringLiteral() {
-  //     cipher_padding(this.(StringLiteral).getValue().splitAt("/"))
-  //   }
-  //   override string toString() { result = this.(StringLiteral).toString() }
-  //   override string getValue() {
-  //     result = this.(StringLiteral).getValue().regexpCapture(".*/.*/(.*)", 1)
-  //   }
-  // }
-  /**
-   * A class to represent when AES is used
-   * AND currently it has literal mode and padding provided
-   *
-   * this currently does not capture the use without a literal
-   * though should be extended to
-   */
-  class CipherAlgorithm extends Crypto::SymmetricAlgorithm {
+  class EncryptionAlgorithm extends Crypto::EncryptionAlgorithm {
     CipherStringLiteral origin;
     CipherGetInstanceAlgorithmArg instance;
 
-    CipherAlgorithm() {
-      this = Crypto::TSymmetricAlgorithm(instance) and
+    EncryptionAlgorithm() {
+      this = Crypto::TEncryptionAlgorithm(instance) and
       instance.getOrigin() = origin
     }
 
     override Location getLocation() { result = instance.getLocation() }
 
-    override Crypto::ModeOfOperation getModeOfOperation() {
-      result.(ModeStringLiteral).getInstance() = origin
+    override Crypto::ModeOfOperationAlgorithm getModeOfOperation() {
+      result.(ModeOfOperation).getInstance() = origin
     }
 
     override Crypto::LocatableElement getOrigin(string name) {
@@ -138,23 +148,25 @@ module JCAModel {
 
     override string getRawAlgorithmName() { result = origin.getValue() }
 
-    override Crypto::TSymmetricCipherType getCipherFamily() {
+    override Crypto::TCipherType getCipherFamily() {
       this.cipherNameMapping(result, origin.getAlgorithmName())
     }
 
     override string getKeySize(Location location) { none() }
 
     bindingset[name]
-    private predicate cipherNameMappingKnown(Crypto::TSymmetricCipherType type, string name) {
+    private predicate cipherNameMappingKnown(Crypto::TCipherType type, string name) {
       name = "AES" and
       type instanceof Crypto::AES
       or
       name = "RC4" and
       type instanceof Crypto::RC4
+      // or
+      // TODO
     }
 
     bindingset[name]
-    predicate cipherNameMapping(Crypto::TSymmetricCipherType type, string name) {
+    predicate cipherNameMapping(Crypto::TCipherType type, string name) {
       this.cipherNameMappingKnown(type, name)
       or
       not this.cipherNameMappingKnown(_, name) and

java/ql/src/experimental/Quantum/Test.ql

@@ -4,6 +4,6 @@
 
 import experimental.Quantum.Language
 
-from Crypto::SymmetricAlgorithm a, Crypto::ModeOfOperation mode
+from Crypto::EncryptionAlgorithm a, Crypto::ModeOfOperationAlgorithm mode
 where a.getModeOfOperation() = mode
 select a, a.getAlgorithmName(), a.getRawAlgorithmName(), mode, mode.getAlgorithmName()