Crypto: Misc. refactoring and code clean up.

cpp/ql/lib/experimental/quantum/Language.qll

@@ -94,7 +94,10 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
     // where typical algorithms are specified, but EC specifically means set up a
     // default curve container, that will later be specified explicitly (or if not a default)
     // curve is used.
-    this.getValue() != "EC"
+    this.getValue() != "EC" and
+    // Exclude all 0's as algorithms. Currently we know of no algorithm defined as 0, and
+    // the typical case is 0 is assigned to represent null.
+    this.getValue().toInt() != 0
   }
 
   override DataFlow::Node getOutputNode() { result.asExpr() = this }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll

@@ -3,6 +3,7 @@ private import experimental.quantum.Language
 private import semmle.code.cpp.dataflow.new.DataFlow
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
+private import PaddingAlgorithmInstance
 
 /**
  * Traces 'known algorithms' to AVCs, specifically
@@ -19,6 +20,9 @@ module KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::
   predicate isSink(DataFlow::Node sink) {
     exists(OpenSSLAlgorithmValueConsumer c |
       c.getInputNode() = sink and
+      // exclude padding algorithm consumers, since
+      // these consumers take in different constant values
+      // not in the typical "known algorithm" set
       not c instanceof PaddingAlgorithmValueConsumer
     )
   }
@@ -43,9 +47,7 @@ module KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow =
   DataFlow::Global<KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig>;
 
 module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
-  }
+  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof OpenSSLPaddingLiteral }
 
   predicate isSink(DataFlow::Node sink) {
     exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/BlockAlgorithmInstance.qll

@@ -8,7 +8,7 @@ private import AlgToAVCFlow
 
 /**
  * Given a `KnownOpenSSLBlockModeAlgorithmConstant`, converts this to a block family type.
- * Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
+ * Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
  */
 predicate knownOpenSSLConstantToBlockModeFamilyType(
   KnownOpenSSLBlockModeAlgorithmConstant e, Crypto::TBlockCipherModeOfOperationType type

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll

@@ -11,7 +11,7 @@ private import BlockAlgorithmInstance
 
 /**
  * Given a `KnownOpenSSLCipherAlgorithmConstant`, converts this to a cipher family type.
- * Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
+ * Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
  */
 predicate knownOpenSSLConstantToCipherFamilyType(
   KnownOpenSSLCipherAlgorithmConstant e, Crypto::KeyOpAlg::TAlgorithm type

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll

@@ -1,5 +1,4 @@
 import cpp
-private import experimental.quantum.OpenSSL.LibraryDetector
 
 predicate resolveAlgorithmFromExpr(Expr e, string normalizedName, string algType) {
   resolveAlgorithmFromCall(e, normalizedName, algType)
@@ -20,7 +19,7 @@ class KnownOpenSSLCipherAlgorithmConstant extends KnownOpenSSLAlgorithmConstant
 
   KnownOpenSSLCipherAlgorithmConstant() {
     resolveAlgorithmFromExpr(this, _, algType) and
-    algType.toLowerCase().matches("%encryption")
+    algType.matches("%ENCRYPTION")
   }
 
   int getExplicitKeySize() {
@@ -37,7 +36,7 @@ class KnownOpenSSLPaddingAlgorithmConstant extends KnownOpenSSLAlgorithmConstant
 
   KnownOpenSSLPaddingAlgorithmConstant() {
     resolveAlgorithmFromExpr(this, _, algType) and
-    algType.toLowerCase().matches("%padding")
+    algType.matches("%PADDING")
   }
 }
 
@@ -46,7 +45,7 @@ class KnownOpenSSLBlockModeAlgorithmConstant extends KnownOpenSSLAlgorithmConsta
 
   KnownOpenSSLBlockModeAlgorithmConstant() {
     resolveAlgorithmFromExpr(this, _, algType) and
-    algType.toLowerCase().matches("%block_mode")
+    algType.matches("%BLOCK_MODE")
   }
 }
 
@@ -55,7 +54,7 @@ class KnownOpenSSLHashAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
 
   KnownOpenSSLHashAlgorithmConstant() {
     resolveAlgorithmFromExpr(this, _, algType) and
-    algType.toLowerCase().matches("%hash")
+    algType.matches("%HASH")
   }
 
   int getExplicitDigestLength() {
@@ -71,7 +70,7 @@ class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmCo
   KnownOpenSSLEllipticCurveAlgorithmConstant() {
     exists(string algType |
       resolveAlgorithmFromExpr(this, _, algType) and
-      algType.toLowerCase().matches("elliptic_curve")
+      algType.matches("ELLIPTIC_CURVE")
     )
   }
 }
@@ -89,7 +88,6 @@ class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmCo
  *  alias = "dss1" and target = "dsaWithSHA1"
  */
 predicate resolveAlgorithmFromCall(Call c, string normalized, string algType) {
-  isPossibleOpenSSLFunction(c.getTarget()) and
   exists(string name, string parsedTargetName |
     parsedTargetName =
       c.getTarget().getName().replaceAll("EVP_", "").toLowerCase().replaceAll("_", "-") and

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll

@@ -6,9 +6,26 @@ private import AlgToAVCFlow
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
 
+/**
+ * A class to define padding specific integer values.
+ * from rsa.h in openssl:
+ *     # define RSA_PKCS1_PADDING          1
+ *     # define RSA_NO_PADDING             3
+ *     # define RSA_PKCS1_OAEP_PADDING     4
+ *     # define RSA_X931_PADDING           5
+ *     # define RSA_PKCS1_PSS_PADDING      6
+ *     # define RSA_PKCS1_WITH_TLS_PADDING 7
+ *     # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
+ */
+class OpenSSLPaddingLiteral extends Literal {
+  // TODO: we can be more specific about where the literal is in a larger expression
+  // to avoid literals that are clealy not representing an algorithm, e.g., array indices.
+  OpenSSLPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
+}
+
 /**
  * Given a `KnownOpenSSLPaddingAlgorithmConstant`, converts this to a padding family type.
- * Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
+ * Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
  */
 predicate knownOpenSSLConstantToPaddingFamilyType(
   KnownOpenSSLPaddingAlgorithmConstant e, Crypto::TPaddingType type
@@ -60,19 +77,8 @@ class KnownOpenSSLPaddingConstantAlgorithmInstance extends OpenSSLAlgorithmInsta
     this instanceof KnownOpenSSLPaddingAlgorithmConstant and
     isPaddingSpecificConsumer = false
     or
-    // Possibility 3:
-    // from rsa.h in openssl:
-    // # define RSA_PKCS1_PADDING          1
-    // # define RSA_NO_PADDING             3
-    // # define RSA_PKCS1_OAEP_PADDING     4
-    // # define RSA_X931_PADDING           5
-    // /* EVP_PKEY_ only */
-    // # define RSA_PKCS1_PSS_PADDING      6
-    // # define RSA_PKCS1_WITH_TLS_PADDING 7
-    // /* internal RSA_ only */
-    // # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
-    this instanceof Literal and
-    this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] and
+    // Possibility 3: padding-specific literal
+    this instanceof OpenSSLPaddingLiteral and
     exists(DataFlow::Node src, DataFlow::Node sink |
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
@@ -88,24 +94,24 @@ class KnownOpenSSLPaddingConstantAlgorithmInstance extends OpenSSLAlgorithmInsta
 
   override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
 
+  Crypto::TPaddingType getKnownPaddingType() {
+    this.(Literal).getValue().toInt() in [1, 7, 8] and result = Crypto::PKCS1_v1_5()
+    or
+    this.(Literal).getValue().toInt() = 3 and result = Crypto::NoPadding()
+    or
+    this.(Literal).getValue().toInt() = 4 and result = Crypto::OAEP()
+    or
+    this.(Literal).getValue().toInt() = 5 and result = Crypto::ANSI_X9_23()
+    or
+    this.(Literal).getValue().toInt() = 6 and result = Crypto::PSS()
+  }
+
   override Crypto::TPaddingType getPaddingType() {
     isPaddingSpecificConsumer = true and
     (
-      if this.(Literal).getValue().toInt() in [1, 7, 8]
-      then result = Crypto::PKCS1_v1_5()
-      else
-        if this.(Literal).getValue().toInt() = 3
-        then result = Crypto::NoPadding()
-        else
-          if this.(Literal).getValue().toInt() = 4
-          then result = Crypto::OAEP()
-          else
-            if this.(Literal).getValue().toInt() = 5
-            then result = Crypto::ANSI_X9_23()
-            else
-              if this.(Literal).getValue().toInt() = 6
-              then result = Crypto::PSS()
-              else result = Crypto::OtherPadding()
+      result = getKnownPaddingType()
+      or
+      not exists(getKnownPaddingType()) and result = Crypto::OtherPadding()
     )
     or
     isPaddingSpecificConsumer = false and

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/CipherAlgorithmValueConsumer.qll

@@ -1,6 +1,5 @@
 import cpp
 private import experimental.quantum.Language
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
 private import OpenSSLAlgorithmValueConsumerBase
@@ -14,7 +13,6 @@ class EVPCipherAlgorithmValueConsumer extends CipherAlgorithmValueConsumer {
 
   EVPCipherAlgorithmValueConsumer() {
     resultNode.asExpr() = this and
-    isPossibleOpenSSLFunction(this.(Call).getTarget()) and
     (
       this.(Call).getTarget().getName() in [
           "EVP_get_cipherbyname", "EVP_get_cipherbyobj", "EVP_get_cipherbynid"

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/EllipticCurveAlgorithmValueConsumer.qll

@@ -1,6 +1,5 @@
 import cpp
 private import experimental.quantum.Language
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
 private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
@@ -14,7 +13,6 @@ class EVPEllipticCurveAlgorithmConsumer extends EllipticCurveValueConsumer {
 
   EVPEllipticCurveAlgorithmConsumer() {
     resultNode.asExpr() = this.(Call) and // in all cases the result is the return
-    isPossibleOpenSSLFunction(this.(Call).getTarget()) and
     (
       this.(Call).getTarget().getName() in ["EVP_EC_gen", "EC_KEY_new_by_curve_name"] and
       valueArgNode.asExpr() = this.(Call).getArgument(0)

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/OpenSSLAlgorithmValueConsumerBase.qll

@@ -1,5 +1,4 @@
 private import experimental.quantum.Language
-private import semmle.code.cpp.dataflow.new.DataFlow
 
 abstract class OpenSSLAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer instanceof Call {
   /**

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PKeyAlgorithmValueConsumer.qll

@@ -1,6 +1,5 @@
 import cpp
 private import experimental.quantum.Language
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
 private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
@@ -13,7 +12,6 @@ class EVPPKeyAlgorithmConsumer extends PKeyValueConsumer {
 
   EVPPKeyAlgorithmConsumer() {
     resultNode.asExpr() = this.(Call) and // in all cases the result is the return
-    isPossibleOpenSSLFunction(this.(Call).getTarget()) and
     (
       // NOTE: some of these consumers are themselves key gen operations,
       // in these cases, the operation will be created separately for the same function.

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PaddingAlgorithmValueConsumer.qll

@@ -1,6 +1,5 @@
 import cpp
 private import experimental.quantum.Language
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
 private import OpenSSLAlgorithmValueConsumerBase
@@ -16,11 +15,8 @@ class EVP_PKEY_CTX_set_rsa_padding_AlgorithmValueConsumer extends PaddingAlgorit
 
   EVP_PKEY_CTX_set_rsa_padding_AlgorithmValueConsumer() {
     resultNode.asExpr() = this and
-    isPossibleOpenSSLFunction(this.(Call).getTarget()) and
-    (
-      this.(Call).getTarget().getName() in ["EVP_PKEY_CTX_set_rsa_padding"] and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
-    )
+    this.(Call).getTarget().getName() in ["EVP_PKEY_CTX_set_rsa_padding"] and
+    valueArgNode.asExpr() = this.(Call).getArgument(1)
   }
 
   override DataFlow::Node getResultNode() { result = resultNode }

cpp/ql/lib/experimental/quantum/OpenSSL/CtxFlow.qll

@@ -20,48 +20,74 @@
 
 import semmle.code.cpp.dataflow.new.DataFlow
 
-class CTXType extends Type {
-  CTXType() {
-    // TODO: should we limit this to an openssl path?
-    this.getUnspecifiedType().stripType().getName().matches("evp_%ctx_%st")
-  }
+/**
+ * An openSSL CTX type, which is type for which the stripped underlying type
+ * matches the pattern 'evp_%ctx_%st'.
+ * This includes types like:
+ * - EVP_CIPHER_CTX
+ * - EVP_MD_CTX
+ * - EVP_PKEY_CTX
+ */
+private class CTXType extends Type {
+  CTXType() { this.getUnspecifiedType().stripType().getName().matches("evp_%ctx_%st") }
 }
 
-class CTXPointerExpr extends Expr {
+/**
+ * A pointer to a CTXType
+ */
+private class CTXPointerExpr extends Expr {
   CTXPointerExpr() {
     this.getType() instanceof CTXType and
     this.getType() instanceof PointerType
   }
 }
 
-class CTXPointerArgument extends CTXPointerExpr {
+/**
+ * A call argument of type CTXPointerExpr.
+ */
+private class CTXPointerArgument extends CTXPointerExpr {
   CTXPointerArgument() { exists(Call c | c.getAnArgument() = this) }
 
   Call getCall() { result.getAnArgument() = this }
 }
 
-class CTXClearCall extends Call {
+/**
+ * A call whose target contains 'free' or 'reset' and has an argument of type
+ * CTXPointerArgument.
+ */
+private class CTXClearCall extends Call {
   CTXClearCall() {
     this.getTarget().getName().toLowerCase().matches(["%free%", "%reset%"]) and
     this.getAnArgument() instanceof CTXPointerArgument
   }
 }
 
-class CTXCopyOutArgCall extends Call {
+/**
+ * A call whose target contains 'copy' and has an argument of type
+ * CTXPointerArgument.
+ */
+private class CTXCopyOutArgCall extends Call {
   CTXCopyOutArgCall() {
-    this.getTarget().getName().toLowerCase().matches(["%copy%"]) and
+    this.getTarget().getName().toLowerCase().matches("%copy%") and
     this.getAnArgument() instanceof CTXPointerArgument
   }
 }
 
-class CTXCopyReturnCall extends Call {
+/**
+ * A call whose target contains 'dup' and has an argument of type
+ * CTXPointerArgument.
+ */
+private class CTXCopyReturnCall extends Call {
   CTXCopyReturnCall() {
-    this.getTarget().getName().toLowerCase().matches(["%dup%"]) and
+    this.getTarget().getName().toLowerCase().matches("%dup%") and
     this.getAnArgument() instanceof CTXPointerArgument and
     this instanceof CTXPointerExpr
   }
 }
 
+/**
+ * Flow from any CTXPointerArgument to any other CTXPointerArgument
+ */
 module OpenSSLCTXArgumentFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr() instanceof CTXPointerArgument }
 
@@ -90,6 +116,9 @@ module OpenSSLCTXArgumentFlowConfig implements DataFlow::ConfigSig {
 
 module OpenSSLCTXArgumentFlow = DataFlow::Global<OpenSSLCTXArgumentFlowConfig>;
 
+/**
+ * Holds if there is a context flow from the source to the sink.
+ */
 predicate ctxArgFlowsToCtxArg(CTXPointerArgument source, CTXPointerArgument sink) {
   exists(DataFlow::Node a, DataFlow::Node b |
     OpenSSLCTXArgumentFlow::flow(a, b) and

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

@@ -1,10 +1,6 @@
-import cpp
-import semmle.code.cpp.dataflow.new.DataFlow
-
 module OpenSSLModel {
-  import experimental.quantum.Language
-  import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
-  import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
-  import experimental.quantum.OpenSSL.Operations.OpenSSLOperations
-  import experimental.quantum.OpenSSL.Random
+  import AlgorithmInstances.OpenSSLAlgorithmInstances
+  import AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
+  import Operations.OpenSSLOperations
+  import Random
 }

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll

@@ -1,5 +1,4 @@
 private import experimental.quantum.Language
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
 private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
@@ -18,10 +17,7 @@ private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
 private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsumerConfig>;
 
 class ECKeyGenOperation extends OpenSSLOperation, Crypto::KeyGenerationOperationInstance {
-  ECKeyGenOperation() {
-    this.(Call).getTarget().getName() = "EC_KEY_generate_key" and
-    isPossibleOpenSSLFunction(this.(Call).getTarget())
-  }
+  ECKeyGenOperation() { this.(Call).getTarget().getName() = "EC_KEY_generate_key" }
 
   override Expr getOutputArg() {
     result = this.(Call) // return value of call

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll

@@ -4,7 +4,6 @@
 
 private import experimental.quantum.Language
 private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
-private import experimental.quantum.OpenSSL.LibraryDetector
 private import OpenSSLOperationBase
 private import EVPHashInitializer
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
@@ -42,10 +41,7 @@ private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsu
 
 //https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis
 class EVP_Q_Digest_Operation extends EVP_Hash_Operation {
-  EVP_Q_Digest_Operation() {
-    this.(Call).getTarget().getName() = "EVP_Q_digest" and
-    isPossibleOpenSSLFunction(this.(Call).getTarget())
-  }
+  EVP_Q_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
 
   //override Crypto::AlgorithmConsumer getAlgorithmConsumer() {  }
   override EVP_Hash_Initializer getInitCall() {
@@ -71,10 +67,7 @@ class EVP_Q_Digest_Operation extends EVP_Hash_Operation {
 }
 
 class EVP_Digest_Operation extends EVP_Hash_Operation {
-  EVP_Digest_Operation() {
-    this.(Call).getTarget().getName() = "EVP_Digest" and
-    isPossibleOpenSSLFunction(this.(Call).getTarget())
-  }
+  EVP_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Digest" }
 
   // There is no context argument for this function
   override Expr getContextArg() { none() }