Added support for bare symmetric algorithms

Author: fegge

csharp/ql/lib/experimental/quantum/dotnet/AlgorithmInstances.qll

@@ -46,47 +46,82 @@ class HashAlgorithmNameInstance extends Crypto::HashAlgorithmInstance instanceof
   Crypto::AlgorithmValueConsumer getConsumer() { result = consumer }
 }
 
-class SymmetricAlgorithmInstance extends Crypto::KeyOperationAlgorithmInstance instanceof SymmetricAlgorithmCreation
+/**
+ * A call to an encryption, decryption, or transform creation API (e.g.
+ * `EncryptCbc` or `CreateEncryptor`) on a `SymmetricAlgorithm` instance.
+ */
+class SymmetricAlgorithmInstance extends Crypto::KeyOperationAlgorithmInstance instanceof SymmetricAlgorithmUse
 {
-  SymmetricAlgorithmConsumer consumer;
-
-  SymmetricAlgorithmInstance() { consumer = SymmetricAlgorithmFlow::getUseFromCreation(this, _, _) }
+  SymmetricAlgorithmInstance() {
+    this.isEncryptionCall() or this.isDecryptionCall() or this.isCreationCall()
+  }
 
-  override string getRawAlgorithmName() { result = super.getSymmetricAlgorithm().getName() }
+  override string getRawAlgorithmName() {
+    result = super.getSymmetricAlgorithm().getType().getName()
+  }
 
   override Crypto::KeyOpAlg::Algorithm getAlgorithmType() {
     if exists(symmetricAlgorithmNameToType(this.getRawAlgorithmName()))
     then result = symmetricAlgorithmNameToType(this.getRawAlgorithmName())
     else result = Crypto::KeyOpAlg::TSymmetricCipher(Crypto::KeyOpAlg::OtherSymmetricCipherType())
   }
 
-  // The cipher mode is set by assigning it to the `Mode` property of the
-  // symmetric algorithm.
   override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() {
+    super.isCreationCall() and
     result.(CipherModeLiteralInstance).getConsumer() = this.getCipherModeAlgorithmValueConsumer()
+    or
+    (super.isEncryptionCall() or super.isDecryptionCall()) and
+    result = this
   }
 
-  // The padding mode is set by assigning it to the `Padding` property of the
-  // symmetric algorithm.
   override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() {
     result.(PaddingModeLiteralInstance).getConsumer() = this.getPaddingAlgorithmValueConsumer()
   }
 
+  // The padding mode is set by assigning it to the `Padding` property of the
+  // symmetric algorithm. It can also be passed as an argument to `EncryptCbc`,
+  // `EncryptCfb`, etc.
   Crypto::AlgorithmValueConsumer getPaddingAlgorithmValueConsumer() {
-    result = SymmetricAlgorithmFlow::getUseFromCreation(this, _, _) and
+    super.isCreationCall() and
+    result = SymmetricAlgorithmFlow::getIntermediateUseFromUse(this, _, _) and
     result instanceof PaddingPropertyWrite
+    or
+    (super.isEncryptionCall() or super.isDecryptionCall()) and
+    result = super.getPaddingArg()
   }
 
+  // The cipher mode is set by assigning it to the `Mode` property of the
+  // symmetric algorithm, or if this is an encryption/decryption call, it
+  // is implicit in the method name.
   Crypto::AlgorithmValueConsumer getCipherModeAlgorithmValueConsumer() {
-    result = SymmetricAlgorithmFlow::getUseFromCreation(this, _, _) and
+    result = SymmetricAlgorithmFlow::getIntermediateUseFromUse(this, _, _) and
     result instanceof CipherModePropertyWrite
   }
 
   override int getKeySizeFixed() { none() }
 
   override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() { none() }
+}
 
-  Crypto::AlgorithmValueConsumer getConsumer() { result = consumer }
+/**
+ * A call to an encryption or decryption API (e.g. `EncryptCbc` or `EncryptCfb`)
+ * on a `SymmetricAlgorithm` instance.
+ *
+ * For these, the cipher mode is given by the method name.
+ */
+class SymmetricAlgorithmMode extends Crypto::ModeOfOperationAlgorithmInstance instanceof SymmetricAlgorithmUse
+{
+  SymmetricAlgorithmMode() { this.isEncryptionCall() or this.isDecryptionCall() }
+
+  override string getRawModeAlgorithmName() {
+    result = this.(SymmetricAlgorithmUse).getRawModeAlgorithmName()
+  }
+
+  override Crypto::TBlockCipherModeOfOperationType getModeType() {
+    if exists(modeNameToType(this.getRawModeAlgorithmName().toUpperCase()))
+    then result = modeNameToType(this.getRawModeAlgorithmName().toUpperCase())
+    else result = Crypto::OtherMode()
+  }
 }
 
 /**
@@ -113,7 +148,7 @@ class PaddingModeLiteralInstance extends Crypto::PaddingAlgorithmInstance instan
 }
 
 /**
- * A padding mode literal, such as `PaddingMode.PKCS7`.
+ * A cipher mode literal, such as `CipherMode.CBC`.
  */
 class CipherModeLiteralInstance extends Crypto::ModeOfOperationAlgorithmInstance instanceof MemberConstantAccess
 {

csharp/ql/lib/experimental/quantum/dotnet/AlgorithmValueConsumers.qll

@@ -44,18 +44,39 @@ class CipherModePropertyWrite extends Crypto::AlgorithmValueConsumer instanceof
 }
 
 /**
- * A call to a `SymmetricAlgorithm.CreateEncryptor` or `SymmetricAlgorithm.CreateDecryptor`
- * method that returns a `CryptoTransform` instance.
+ * A padding mode argument passed to a symmetric algorithm method call.
  */
-class SymmetricAlgorithmConsumer extends Crypto::AlgorithmValueConsumer instanceof CryptoTransformCreation
+class PaddingModeArgument extends Crypto::AlgorithmValueConsumer instanceof Expr {
+  SymmetricAlgorithmUse use;
+
+  PaddingModeArgument() {
+    (use.isEncryptionCall() or use.isDecryptionCall()) and
+    this = use.getPaddingArg()
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getInputNode() { result.asExpr() = this }
+
+  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
+    result.(PaddingModeLiteralInstance).getConsumer() = this
+  }
+}
+
+/**
+ * A qualified expression where the qualifier is a `SymmetricAlgorithm`
+ * instance. (e.g. a call to `SymmetricAlgorithm.EncryptCbc` or
+ * `SymmetricAlgorithm.CreateEncryptor`)
+ */
+class SymmetricAlgorithmConsumer extends Crypto::AlgorithmValueConsumer instanceof SymmetricAlgorithmUse
 {
+  SymmetricAlgorithmConsumer() {
+    super.isEncryptionCall() or super.isDecryptionCall() or super.isCreationCall()
+  }
+
   override Crypto::ConsumerInputDataFlowNode getInputNode() {
     result.asExpr() = super.getQualifier()
   }
 
-  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
-    result.(SymmetricAlgorithmInstance).getConsumer() = this
-  }
+  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
 }
 
 /**

csharp/ql/lib/experimental/quantum/dotnet/Cryptography.qll

@@ -423,21 +423,23 @@ class SymmetricAlgorithmCreation extends MethodCall {
     this.getTarget().hasName("Create") and
     this.getQualifier().getType() instanceof SymmetricAlgorithm
   }
-
-  SymmetricAlgorithm getSymmetricAlgorithm() { result = this.getQualifier().getType() }
 }
 
 class SymmetricAlgorithmUse extends QualifiableExpr {
   SymmetricAlgorithmUse() {
     this.getQualifier().getType() instanceof SymmetricAlgorithm and
     this.getQualifiedDeclaration()
-        .hasName(["CreateEncryptor", "CreateDecryptor", "Key", "IV", "Padding", "Mode"])
+        .hasName([
+            "EncryptCbc", "DecryptCbc", "EncryptCfb", "DecryptCfb", "EncryptEcb", "DecryptEcb",
+            "TryEncryptCbc", "TryDecryptCbc", "TryEncryptCfb", "TryDecryptCfb", "TryEncryptEcb",
+            "TryDecryptEcb", "CreateEncryptor", "CreateDecryptor", "Key", "IV", "Padding", "Mode"
+          ])
   }
 
   Expr getSymmetricAlgorithm() { result = this.getQualifier() }
 
   predicate isIntermediate() {
-    not this.getQualifiedDeclaration().hasName(["CreateEncryptor", "CreateDecryptor"])
+    this.getQualifiedDeclaration().hasName(["Key", "IV", "Padding", "Mode"])
   }
 
   // The key may be set by assigning it to the `Key` property of the symmetric algorithm.
@@ -459,6 +461,50 @@ class SymmetricAlgorithmUse extends QualifiableExpr {
   predicate isModeConsumer() {
     this instanceof PropertyWrite and this.getQualifiedDeclaration().getName() = "Mode"
   }
+
+  predicate isCreationCall() {
+    // TODO: Matching using `hasName` does not work here for some reason.
+    this.getQualifiedDeclaration().getName().matches("Create%")
+  }
+
+  predicate isEncryptionCall() {
+    // TODO: Matching using `hasName` does not work here for some reason.
+    this.getQualifiedDeclaration().getName().matches(["Encrypt%", "TryEncrypt%"])
+  }
+
+  predicate isDecryptionCall() {
+    // TODO: Matching using `hasName` does not work here for some reason.
+    this.getQualifiedDeclaration().getName().matches(["Decrypt%", "TryDecrypt%"])
+  }
+
+  string getRawModeAlgorithmName() {
+    this.isEncryptionCall() and
+    result = this.getQualifiedDeclaration().getName().splitAt("Encrypt", 1)
+    or
+    this.isDecryptionCall() and
+    result = this.getQualifiedDeclaration().getName().splitAt("Decrypt", 1)
+  }
+
+  Expr getInputArg() {
+    (this.isEncryptionCall() or this.isDecryptionCall()) and
+    result = this.(MethodCall).getArgument(0)
+  }
+
+  Expr getIvArg() {
+    (this.isEncryptionCall() or this.isDecryptionCall()) and
+    this.getRawModeAlgorithmName().matches(["Cbc", "Cfb"]) and
+    result = this.(MethodCall).getArgument(1)
+  }
+
+  Expr getPaddingArg() {
+    (this.isEncryptionCall() or this.isDecryptionCall()) and
+    result = this.(MethodCall).getArgument(this.(MethodCall).getNumberOfArguments() - 1)
+  }
+
+  Expr getOutput() {
+    (this.isEncryptionCall() or this.isDecryptionCall()) and
+    result = this
+  }
 }
 
 /**

csharp/ql/lib/experimental/quantum/dotnet/FlowAnalysis.qll

@@ -151,6 +151,7 @@ module ModeLiteralFlow {
 
     predicate isSink(DataFlow::Node sink) {
       sink.asExpr() instanceof PaddingPropertyWrite or
+      sink.asExpr() instanceof PaddingModeArgument or
       sink.asExpr() instanceof CipherModePropertyWrite
     }
 
@@ -165,9 +166,7 @@ module ModeLiteralFlow {
 
   private module ModeLiteralFlow = DataFlow::Global<ModeLiteralConfig>;
 
-  SymmetricAlgorithmUse getConsumer(
-    Expr mode, ModeLiteralFlow::PathNode source, ModeLiteralFlow::PathNode sink
-  ) {
+  Expr getConsumer(Expr mode, ModeLiteralFlow::PathNode source, ModeLiteralFlow::PathNode sink) {
     source.getNode().asExpr() = mode and
     sink.getNode().asExpr() = result and
     ModeLiteralFlow::flowPath(source, sink)

csharp/ql/lib/experimental/quantum/dotnet/OperationInstances.qll

@@ -55,27 +55,57 @@ class HashOperationInstance extends Crypto::HashOperationInstance instanceof Has
   }
 }
 
+/**
+ * A call to an encryption or decryption API (e.g. `EncryptCbc` or `EncryptCfb`)
+ * on a `SymmetricAlgorithm` instance.
+ */
+class SymmetricAlgorithmOperationInstance extends Crypto::KeyOperationInstance instanceof SymmetricAlgorithmUse
+{
+  SymmetricAlgorithmOperationInstance() { super.isEncryptionCall() or super.isDecryptionCall() }
+
+  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { result = this }
+
+  override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
+    if super.isEncryptionCall()
+    then result = Crypto::TEncryptMode()
+    else
+      if super.isDecryptionCall()
+      then result = Crypto::TDecryptMode()
+      else result = Crypto::TUnknownKeyOperationMode()
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getKeyConsumer() {
+    result.asExpr() = SymmetricAlgorithmFlow::getIntermediateUseFromUse(this, _, _) and
+    result.asExpr().(SymmetricAlgorithmUse).isKeyConsumer()
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getNonceConsumer() {
+    result.asExpr() = super.getIvArg()
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    result.asExpr() = super.getInputArg()
+  }
+
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    result.asExpr() = super.getOutput()
+  }
+}
+
 /**
  * An instantiation of a `CryptoStream` object where the transform is a symmetric
  * encryption or decryption operation (e.g. an encryption transform created by a
  * call to `Aes.CreateEncryptor()`)
  */
 class CryptoStreamOperationInstance extends Crypto::KeyOperationInstance instanceof CryptoStreamCreation
 {
-  CryptoTransformCreation transform;
-
-  CryptoStreamOperationInstance() {
-    transform = CryptoTransformFlow::getCreationFromUse(this) and
-    (transform.isEncryptor() or transform.isDecryptor())
-  }
-
-  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { result = transform }
+  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { result = this.getCryptoTransform() }
 
   override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
-    if transform.isEncryptor()
+    if this.getCryptoTransform().isEncryptor()
     then result = Crypto::TEncryptMode()
     else
-      if transform.isDecryptor()
+      if this.getCryptoTransform().isDecryptor()
       then result = Crypto::TDecryptMode()
       else result = Crypto::TUnknownKeyOperationMode()
   }
@@ -84,10 +114,10 @@ class CryptoStreamOperationInstance extends Crypto::KeyOperationInstance instanc
     // If a key is explicitly provided as an argument when the transform is
     // created, this takes precedence over any key that may be set on the
     // symmetric algorithm instance.
-    if exists(transform.getKeyArg())
-    then result.asExpr() = transform.getKeyArg()
+    if exists(this.getCryptoTransform().getKeyArg())
+    then result.asExpr() = this.getCryptoTransform().getKeyArg()
     else (
-      result.asExpr() = SymmetricAlgorithmFlow::getIntermediateUseFromUse(transform, _, _) and
+      result.asExpr() = SymmetricAlgorithmFlow::getIntermediateUseFromUse(this.getCryptoTransform(), _, _) and
       result.asExpr().(SymmetricAlgorithmUse).isKeyConsumer()
     )
   }
@@ -96,10 +126,10 @@ class CryptoStreamOperationInstance extends Crypto::KeyOperationInstance instanc
     // If an IV is explicitly provided as an argument when the transform is
     // created, this takes precedence over any IV that may be set on the
     // symmetric algorithm instance.
-    if exists(transform.getIvArg())
-    then result.asExpr() = transform.getIvArg()
+    if exists(this.getCryptoTransform().getIvArg())
+    then result.asExpr() = this.getCryptoTransform().getIvArg()
     else (
-      result.asExpr() = SymmetricAlgorithmFlow::getIntermediateUseFromUse(transform, _, _) and
+      result.asExpr() = SymmetricAlgorithmFlow::getIntermediateUseFromUse(this.getCryptoTransform(), _, _) and
       result.asExpr().(SymmetricAlgorithmUse).isIvConsumer()
     )
   }
@@ -126,6 +156,11 @@ class CryptoStreamOperationInstance extends Crypto::KeyOperationInstance instanc
     result.asExpr() = this.getLaterWrappedStreamUse().getOutput()
   }
 
+  CryptoTransformCreation getCryptoTransform() {
+    result = CryptoTransformFlow::getCreationFromUse(this) and
+    (result.isEncryptor() or result.isDecryptor())
+  }
+
   // Gets either this stream, or a stream wrapped by this stream.
   StreamCreation getWrappedStreamCreation() {
     result = StreamFlow::getWrappedStreamCreation(this, _, _)