Continue Artifact data-flow WIP

nicolaswill · nicolaswill · commit 8707e4d9a394 · 2025-02-18T18:35:49.000+01:00
diff --git a/java/ql/lib/experimental/Quantum/JCA.qll b/java/ql/lib/experimental/Quantum/JCA.qll
@@ -244,24 +244,29 @@ module JCAModel {
   /**
    * Initialiation vectors
    */
-  abstract class IVParameterInstantiation extends ClassInstanceExpr {
-    abstract Expr getIV();
+  abstract class IVParameterInstantiation extends Crypto::InitializationVectorArtifactInstance instanceof ClassInstanceExpr
+  {
+    abstract Expr getInput();
   }
 
   class IvParameterSpecInstance extends IVParameterInstantiation {
     IvParameterSpecInstance() {
-      this.getConstructedType().hasQualifiedName("javax.crypto.spec", "IvParameterSpec")
+      this.(ClassInstanceExpr)
+          .getConstructedType()
+          .hasQualifiedName("javax.crypto.spec", "IvParameterSpec")
     }
 
-    override Expr getIV() { result = super.getArgument(0) }
+    override Expr getInput() { result = this.(ClassInstanceExpr).getArgument(0) }
   }
 
   class GCMParameterSpecInstance extends IVParameterInstantiation {
     GCMParameterSpecInstance() {
-      this.getConstructedType().hasQualifiedName("javax.crypto.spec", "GCMParameterSpec")
+      this.(ClassInstanceExpr)
+          .getConstructedType()
+          .hasQualifiedName("javax.crypto.spec", "GCMParameterSpec")
     }
 
-    override Expr getIV() { result = super.getArgument(1) }
+    override Expr getInput() { result = this.(ClassInstanceExpr).getArgument(1) }
   }
 
   class CipherInitCall extends MethodCall {
@@ -280,18 +285,24 @@ module JCAModel {
   }
 
   // TODO: cipher.getParameters().getParameterSpec(GCMParameterSpec.class);
-  class InitializationVectorExpr extends Crypto::InitializationVectorArtifactInstance instanceof Expr
-  {
-    CipherInitCall call; // TODO: add origin to known sources (e.g. RNG, etc.)
-
-    InitializationVectorExpr() { this = call.getIV() }
-  }
+  /*
+   *  class InitializationVectorArg extends Crypto::InitializationVectorArtifactInstance instanceof Expr
+   *  {
+   *    IVParameterInstantiation creation;
+   *
+   *    InitializationVectorArg() { this = creation.getInput() }
+   *  }
+   */
 
   class InitializationVector extends Crypto::InitializationVector {
-    InitializationVectorExpr instance;
+    IVParameterInstantiation instance;
 
     InitializationVector() { this = Crypto::TInitializationVector(instance) }
 
     override Location getLocation() { result = instance.getLocation() }
+
+    override Crypto::DataFlowNode asOutputData() { result.asExpr() = instance }
+
+    override Crypto::DataFlowNode getInputData() { result.asExpr() = instance.getInput() }
   }
 }
diff --git a/java/ql/lib/experimental/Quantum/Language.qll b/java/ql/lib/experimental/Quantum/Language.qll
@@ -1,30 +1,84 @@
 private import codeql.cryptography.Model
-private import java as Lang
+private import java as Language
+private import semmle.code.java.security.InsecureRandomnessQuery
+private import semmle.code.java.security.RandomQuery
+
+private class UnknownLocation extends Language::Location {
+  UnknownLocation() { this.getFile().getAbsolutePath() = "" }
+}
 
 /**
  * A dummy location which is used when something doesn't have a location in
  * the source code but needs to have a `Location` associated with it. There
  * may be several distinct kinds of unknown locations. For example: one for
  * expressions, one for statements and one for other program elements.
  */
-class UnknownLocation extends Location {
-  UnknownLocation() { this.getFile().getAbsolutePath() = "" }
+private class UnknownDefaultLocation extends UnknownLocation {
+  UnknownDefaultLocation() { locations_default(this, _, 0, 0, 0, 0) }
+}
+
+module CryptoInput implements InputSig<Language::Location> {
+  class DataFlowNode = DataFlow::Node;
+
+  class LocatableElement = Language::Element;
+
+  class UnknownLocation = UnknownDefaultLocation;
+
+  predicate rngToIvFlow(DataFlowNode rng, DataFlowNode iv) { none() }
 }
 
 /**
- * A dummy location which is used when something doesn't have a location in
- * the source code but needs to have a `Location` associated with it.
+ * Instantiate the model
  */
-class UnknownDefaultLocation extends UnknownLocation {
-  UnknownDefaultLocation() { locations_default(this, _, 0, 0, 0, 0) }
+module Crypto = CryptographyBase<Language::Location, CryptoInput>;
+
+/**
+ * Random number generation, where each instance is modelled as the expression
+ * tied to an output node (i.e., the result of the source of randomness)
+ */
+abstract class RandomnessInstance extends Crypto::RandomNumberGenerationInstance {
+  abstract Crypto::RNGSourceSecurity getSourceSecurity();
+
+  Crypto::TRNGSeedSecurity getSeedSecurity(Location location) { none() }
 }
 
-module CryptoInput implements InputSig<Lang::Location> {
-  class LocatableElement = Lang::Element;
+class SecureRandomnessInstance extends RandomnessInstance {
+  SecureRandomnessInstance() {
+    exists(RandomDataSource s | this = s.getOutput() |
+      s.getSourceOfRandomness() instanceof SecureRandomNumberGenerator
+    )
+  }
 
-  class UnknownLocation = UnknownDefaultLocation;
+  override Crypto::RNGSourceSecurity getSourceSecurity() {
+    result instanceof Crypto::RNGSourceSecure
+  }
+}
+
+class InsecureRandomnessInstance extends RandomnessInstance {
+  InsecureRandomnessInstance() { exists(InsecureRandomnessSource node | this = node.asExpr()) }
+
+  override Crypto::RNGSourceSecurity getSourceSecurity() {
+    result instanceof Crypto::RNGSourceInsecure
+  }
 }
 
-module Crypto = CryptographyBase<Lang::Location, CryptoInput>;
+class RandomnessArtifact extends Crypto::RandomNumberGeneration {
+  RandomnessInstance instance;
+
+  RandomnessArtifact() { this = Crypto::TRandomNumberGeneration(instance) }
+
+  override Location getLocation() { result = instance.getLocation() }
+
+  override Crypto::RNGSourceSecurity getSourceSecurity() { result = instance.getSourceSecurity() }
+
+  override Crypto::TRNGSeedSecurity getSeedSecurity(Location location) {
+    result = instance.getSeedSecurity(location)
+  }
+
+  override Crypto::DataFlowNode asOutputData() { result.asExpr() = instance }
+
+  override Crypto::DataFlowNode getInputData() { none() }
+}
 
+// Import library-specific modeling
 import JCA
diff --git a/shared/cryptography/codeql/cryptography/Model.qll b/shared/cryptography/codeql/cryptography/Model.qll
@@ -12,14 +12,24 @@ signature module InputSig<LocationSig Location> {
     string toString();
   }
 
+  class DataFlowNode {
+    Location getLocation();
+
+    string toString();
+  }
+
   class UnknownLocation instanceof Location;
+
+  predicate rngToIvFlow(DataFlowNode rng, DataFlowNode iv);
 }
 
 module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
   final class LocatableElement = Input::LocatableElement;
 
   final class UnknownLocation = Input::UnknownLocation;
 
+  final class DataFlowNode = Input::DataFlowNode;
+
   final class UnknownPropertyValue extends string {
     UnknownPropertyValue() { this = "<unknown>" }
   }
@@ -93,12 +103,15 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
 
   abstract class NonceArtifactInstance extends LocatableElement { }
 
+  abstract class RandomNumberGenerationInstance extends LocatableElement { }
+
   newtype TNode =
     // Artifacts (data that is not an operation or algorithm, e.g., a key)
     TDigest(DigestArtifactInstance e) or
     TKey(KeyArtifactInstance e) or
     TInitializationVector(InitializationVectorArtifactInstance e) or
     TNonce(NonceArtifactInstance e) or
+    TRandomNumberGeneration(RandomNumberGenerationInstance e) or
     // Operations (e.g., hashing, encryption)
     THashOperation(HashOperationInstance e) or
     TKeyDerivationOperation(KeyDerivationOperationInstance e) or
@@ -115,7 +128,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
     TPaddingAlgorithm(PaddingAlgorithmInstance e) or
     // Composite and hybrid cryptosystems (e.g., RSA-OAEP used with AES, post-quantum hybrid cryptosystems)
     // These nodes are always parent nodes and are not modeled but rather defined via library-agnostic patterns.
-    TKemDemHybridCryptosystem(EncryptionAlgorithmInstance dem) or // TODO, change this relation and the below ones
+    TKemDemHybridCryptosystem(EncryptionAlgorithm dem) or // TODO, change this relation and the below ones
     TKeyAgreementHybridCryptosystem(EncryptionAlgorithmInstance ka) or
     TAsymmetricEncryptionMacHybridCryptosystem(EncryptionAlgorithmInstance enc) or
     TPostQuantumHybridCryptosystem(EncryptionAlgorithmInstance enc)
@@ -127,9 +140,9 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
    */
   abstract class NodeBase extends TNode {
     /**
-     * Returns a string representation of this node, usually the name of the operation/algorithm/property.
+     * Returns a string representation of this node.
      */
-    abstract string toString();
+    string toString() { result = this.getInternalType() }
 
     /**
      * Returns a string representation of the internal type of this node, usually the name of the class.
@@ -172,15 +185,48 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
 
   class Asset = NodeBase;
 
-  class Artifact = NodeBase;
+  abstract class Artifact extends NodeBase {
+    abstract DataFlowNode asOutputData();
+
+    abstract DataFlowNode getInputData();
+  }
 
   /**
    * An initialization vector
    */
-  abstract class InitializationVector extends Asset, TInitializationVector {
+  abstract class InitializationVector extends Artifact, TInitializationVector {
     final override string getInternalType() { result = "InitializationVector" }
 
-    final override string toString() { result = this.getInternalType() }
+    RandomNumberGeneration getRNGSource() {
+      Input::rngToIvFlow(result.asOutputData(), this.getInputData())
+    }
+  }
+
+  newtype TRNGSourceSecurity =
+    RNGSourceSecure() or // Secure RNG source (unrelated to seed)
+    RNGSourceInsecure() // Insecure RNG source (unrelated to seed)
+
+  class RNGSourceSecurity extends TRNGSourceSecurity {
+    string toString() {
+      this instanceof RNGSourceSecure and result = "Secure RNG Source"
+      or
+      this instanceof RNGSourceInsecure and result = "Insecure RNG Source"
+    }
+  }
+
+  newtype TRNGSeedSecurity =
+    RNGSeedSecure() or
+    RNGSeedInsecure()
+
+  /**
+   * A source of random number generation
+   */
+  abstract class RandomNumberGeneration extends Artifact, TRandomNumberGeneration {
+    final override string getInternalType() { result = "RandomNumberGeneration" }
+
+    abstract RNGSourceSecurity getSourceSecurity();
+
+    abstract TRNGSeedSecurity getSeedSecurity(Location location);
   }
 
   /**
@@ -197,8 +243,6 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
      */
     abstract string getOperationType();
 
-    final override string toString() { result = this.getOperationType() }
-
     final override string getInternalType() { result = this.getOperationType() }
 
     override NodeBase getChild(string edgeName) {
@@ -210,8 +254,6 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
   }
 
   abstract class Algorithm extends Asset {
-    final override string toString() { result = this.getAlgorithmType() }
-
     final override string getInternalType() { result = this.getAlgorithmType() }
 
     /**
