Making various changes based on the feedback. Pending: 2 non-trivial fixes for Java & Python.

Author: raulgarciamsft

csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.cs

@@ -1,4 +1,15 @@
 
+{
+    SymmetricKey aesKey = new SymmetricKey(kid: "symencryptionkey");
+
+    // BAD: Using the outdated client side encryption version V1_0
+    BlobEncryptionPolicy uploadPolicy = new BlobEncryptionPolicy(key: aesKey, keyResolver: null);
+    BlobRequestOptions uploadOptions = new BlobRequestOptions() { EncryptionPolicy = uploadPolicy };
+
+    MemoryStream stream = new MemoryStream(buffer);
+    blob.UploadFromStream(stream, length: size, accessCondition: null, options: uploadOptions);
+}
+
 var client = new BlobClient(myConnectionString, new SpecializedBlobClientOptions()
 {
     // BAD: Using an outdated SDK that does not support client side encryption version V2_0

csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp

@@ -21,6 +21,9 @@
     <li>
       <a href="http://aka.ms/azstorageclientencryptionblog">Azure Storage Client Encryption Blog.</a>
     </li>
+    <li>
+      <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-PENDING">CVE-2022-PENDING</a>
+    </li>
 
   </references>
 </qhelp>

csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql

@@ -33,16 +33,25 @@ predicate isCreatingOutdatedAzureClientSideEncryptionObject(ObjectCreation oc, C
 }
 
 /**
- * Holds if the Azure.Storage assembly for `c` is a version knwon to support
- * version 2+ for client-side encryption and if the argument for the constructor `version`
- * is set to a secure value.
+ * Holds if the Azure.Storage assembly for `c` is a version known to support
+ * version 2+ for client-side encryption
  */
-predicate isObjectCreationSafe(Expr versionExpr, Assembly asm) {
-  // Check if the Azure.Storage assembly version has the fix
+predicate doesAzureStorageAssemblySupportSafeClientSideEncryption(Assembly asm) {
   exists(int versionCompare |
     versionCompare = asm.getVersion().compareTo("12.12.0.0") and
     versionCompare >= 0
   ) and
+  asm.getName() = "Azure.Storage.Common"
+}
+
+/**
+ * Holds if the Azure.Storage assembly for `c` is a version known to support
+ * version 2+ for client-side encryption and if the argument for the constructor `version`
+ * is set to a secure value.
+ */
+predicate isObjectCreationArgumentSafeAndUsingSafeVersionOfAssembly(Expr versionExpr, Assembly asm) {
+  // Check if the Azure.Storage assembly version has the fix
+  doesAzureStorageAssemblySupportSafeClientSideEncryption(asm) and
   // and that the version argument for the constructor is guaranteed to be Version2
   isExprAnAccessToSafeClientSideEncryptionVersionValue(versionExpr)
 }
@@ -56,8 +65,6 @@ predicate isExprAnAccessToSafeClientSideEncryptionVersionValue(Expr e) {
     ec.hasQualifiedName("Azure.Storage.ClientSideEncryptionVersion.V2_0") and
     ec.getAnAccess() = e
   )
-  or
-  e.getValue().toInt() >= 2
 }
 
 from Expr e, Class c, Assembly asm
@@ -66,10 +73,9 @@ where
   (
     exists(Expr e2 |
       isCreatingAzureClientSideEncryptionObject(e, c, e2) and
-      not isObjectCreationSafe(e2, asm)
+      not isObjectCreationArgumentSafeAndUsingSafeVersionOfAssembly(e2, asm)
     )
     or
     isCreatingOutdatedAzureClientSideEncryptionObject(e, c)
   )
-select e,
-  "Unsafe usage of v1 version of Azure Storage client-side encryption (CVE-2022-PENDING). See http://aka.ms/azstorageclientencryptionblog"
+select e, "Unsafe usage of v1 version of Azure Storage client-side encryption."

java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp

@@ -14,18 +14,16 @@
   </recommendation>
   <example>
 
-    <p>The following example shows an HTTP request parameter being used directly in a forming a
-new request without validating the input, which facilitates SSRF attacks.
-It also shows how to remedy the problem by validating the user input against a known fixed string.
-</p>
-
     <sample src="UnsafeUsageOfClientSideEncryptionVersion.java" />
 
   </example>
   <references>
     <li>
       <a href="http://aka.ms/azstorageclientencryptionblog">Azure Storage Client Encryption Blog.</a>
     </li>
+    <li>
+      <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-PENDING">CVE-2022-PENDING</a>
+    </li>
 
   </references>
 </qhelp>

java/ql/src/experimental/Security/CWE/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql

@@ -13,7 +13,7 @@
 import java
 
 /**
- * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder` 
+ * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder`
  * that takes no arguments, which means that it is using V1 encryption
  */
 predicate isCreatingOutdatedAzureClientSideEncryptionObject(Call call, Class c) {
@@ -31,8 +31,8 @@ predicate isCreatingOutdatedAzureClientSideEncryptionObject(Call call, Class c)
   )
 }
 
-/** 
- * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder` 
+/**
+ * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder`
  * that takes `versionArg` as the argument for the version.
  */
 predicate isCreatingAzureClientSideEncryptionObjectNewVersion(Call call, Class c, Expr versionArg) {
@@ -47,7 +47,7 @@ predicate isCreatingAzureClientSideEncryptionObjectNewVersion(Call call, Class c
 }
 
 /**
- * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder` 
+ * Holds if the call `call` is an object creation for a class `EncryptedBlobClientBuilder`
  * that takes `versionArg` as the argument for the version, and the version number is safe
  */
 predicate isCreatingSafeAzureClientSideEncryptionObject(Call call, Class c, Expr versionArg) {
@@ -67,5 +67,4 @@ where
   )
   or
   isCreatingOutdatedAzureClientSideEncryptionObject(e, c)
-select e,
-  "Unsafe usage of v1 version of Azure Storage client-side encryption (CVE-2022-PENDING). See http://aka.ms/azstorageclientencryptionblog"
+select e, "Unsafe usage of v1 version of Azure Storage client-side encryption."

python/ql/src/experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.py

@@ -1,7 +1,7 @@
 blob_client = blob_service_client.get_blob_client(container=container_name, blob=blob_name)
-    blob_client.require_encryption = True
-    blob_client.key_encryption_key = kek
-    # GOOD: Must use `encryption_version` set to `2.0`
-    blob_client.encryption_version = '2.0'  # Use Version 2.0!
-   with open(“decryptedcontentfile.txt”, “rb”) as stream:
-        blob_client.upload_blob(stream, overwrite=OVERWRITE_EXISTING)
+blob_client.require_encryption = True
+blob_client.key_encryption_key = kek
+# GOOD: Must use `encryption_version` set to `2.0`
+blob_client.encryption_version = '2.0'  # Use Version 2.0!
+with open(“decryptedcontentfile.txt”, “rb”) as stream:
+    blob_client.upload_blob(stream, overwrite=OVERWRITE_EXISTING)

python/ql/src/experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.qhelp

@@ -14,18 +14,16 @@
   </recommendation>
   <example>
 
-    <p>The following example shows an HTTP request parameter being used directly in a forming a
-new request without validating the input, which facilitates SSRF attacks.
-It also shows how to remedy the problem by validating the user input against a known fixed string.
-</p>
-
     <sample src="UnsafeUsageOfClientSideEncryptionVersion.py" />
 
   </example>
   <references>
     <li>
       <a href="http://aka.ms/azstorageclientencryptionblog">Azure Storage Client Encryption Blog.</a>
     </li>
+    <li>
+      <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-PENDING">CVE-2022-PENDING</a>
+    </li>
 
   </references>
 </qhelp>

python/ql/src/experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql

@@ -24,7 +24,7 @@ predicate isUnsafeClientSideAzureStorageEncryptionViaAttributes(Call call, AttrN
     not astmt.getValue() instanceof None and
     not exists(AssignStmt astmt2, Attribute a2, AttrNode encryptionVersionSet, StrConst uc |
       uc = astmt2.getValue() and
-      uc.getLiteralValue().toString() in ["'2.0'", "2.0"] and
+      uc.getText() in ["'2.0'", "2.0"] and
       a2.getAttr() = "encryption_version" and
       a2.getAFlowNode() = encryptionVersionSet and
       encryptionVersionSet.strictlyReaches(ctrlFlowNode)
@@ -34,15 +34,13 @@ predicate isUnsafeClientSideAzureStorageEncryptionViaAttributes(Call call, AttrN
 
 predicate isUnsafeClientSideAzureStorageEncryptionViaObjectCreation(Call call, ControlFlowNode node) {
   exists(Keyword k | k.getAFlowNode() = node |
-    call.getFunc().(Name).getId().toString() in [
-        "ContainerClient", "BlobClient", "BlobServiceClient"
-      ] and
+    call.getFunc().(Name).getId() in ["ContainerClient", "BlobClient", "BlobServiceClient"] and
     k.getArg() = "key_encryption_key" and
     k = call.getANamedArg() and
     not k.getValue() instanceof None and
     not exists(Keyword k2 | k2 = call.getANamedArg() |
       k2.getArg() = "encryption_version" and
-      k2.getValue().(StrConst).getLiteralValue().toString() in ["'2.0'", "2.0"]
+      k2.getValue().(StrConst).getText() in ["'2.0'", "2.0"]
     )
   )
 }
@@ -51,5 +49,4 @@ from Call call, ControlFlowNode node
 where
   isUnsafeClientSideAzureStorageEncryptionViaAttributes(call, node) or
   isUnsafeClientSideAzureStorageEncryptionViaObjectCreation(call, node)
-select node,
-  "Unsafe usage of v1 version of Azure Storage client-side encryption (CVE-2022-PENDING). See http://aka.ms/azstorageclientencryptionblog"
+select node, "Unsafe usage of v1 version of Azure Storage client-side encryption."