Merge pull request #12604 from raulgarciamsft/main

Python: Update `py/azure-storage/unsafe-client-side-encryption-in-use`
2026-05-02 04:05:14 +02:00 · 2023-03-30 11:48:26 +02:00
parent 0202bafc35 f3937a4a12
commit decd51b65d
4 changed files with 293 additions and 72 deletions
--- a/python/ql/src/experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql
+++ b/python/ql/src/experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql
@@ -1,7 +1,7 @@
 /**
 * @name Unsafe usage of v1 version of Azure Storage client-side encryption.
 * @description Using version v1 of Azure Storage client-side encryption is insecure, and may enable an attacker to decrypt encrypted data
- * @kind problem
+ * @kind path-problem
 * @tags security
 *       experimental
 *       cryptography
@@ -12,80 +12,145 @@
 */

 import python
+import semmle.python.dataflow.new.DataFlow
 import semmle.python.ApiGraphs

-predicate isUnsafeClientSideAzureStorageEncryptionViaAttributes(Call call, AttrNode node) {
-  exists(
-    API::Node n, API::Node n2, Attribute a, AssignStmt astmt, API::Node uploadBlob,
-    ControlFlowNode ctrlFlowNode, string s
-  |
-    s in ["key_encryption_key", "key_resolver_function"] and
-    n =
-      API::moduleImport("azure")
-          .getMember("storage")
-          .getMember("blob")
-          .getMember("BlobClient")
-          .getReturn()
-          .getMember(s) and
-    n2 =
-      API::moduleImport("azure")
-          .getMember("storage")
-          .getMember("blob")
-          .getMember("BlobClient")
-          .getReturn()
-          .getMember("upload_blob") and
-    n.getAValueReachableFromSource().asExpr() = a and
-    astmt.getATarget() = a and
-    a.getAFlowNode() = node and
-    uploadBlob =
-      API::moduleImport("azure")
-          .getMember("storage")
-          .getMember("blob")
-          .getMember("BlobClient")
-          .getReturn()
-          .getMember("upload_blob") and
-    uploadBlob.getACall().asExpr() = call and
-    ctrlFlowNode = call.getAFlowNode() and
-    node.strictlyReaches(ctrlFlowNode) and
-    node != ctrlFlowNode and
-    not exists(
-      AssignStmt astmt2, Attribute a2, AttrNode encryptionVersionSet, StrConst uc,
-      API::Node encryptionVersion
-    |
-      uc = astmt2.getValue() and
-      uc.getText() in ["'2.0'", "2.0"] and
-      encryptionVersion =
-        API::moduleImport("azure")
-            .getMember("storage")
-            .getMember("blob")
-            .getMember("BlobClient")
-            .getReturn()
-            .getMember("encryption_version") and
-      encryptionVersion.getAValueReachableFromSource().asExpr() = a2 and
-      astmt2.getATarget() = a2 and
-      a2.getAFlowNode() = encryptionVersionSet and
-      encryptionVersionSet.strictlyReaches(ctrlFlowNode)
-    )
-  )
+API::Node getBlobServiceClient(boolean isSource) {
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("BlobServiceClient")
+        .getReturn()
+  or
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("BlobServiceClient")
+        .getMember("from_connection_string")
+        .getReturn()
 }

-predicate isUnsafeClientSideAzureStorageEncryptionViaObjectCreation(Call call, ControlFlowNode node) {
-  exists(API::Node c, string s, Keyword k | k.getAFlowNode() = node |
-    c.getACall().asExpr() = call and
-    c = API::moduleImport("azure").getMember("storage").getMember("blob").getMember(s) and
-    s 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).getText() in ["'2.0'", "2.0"]
-    )
-  )
+API::CallNode getTransitionToContainerClient() {
+  result = getBlobServiceClient(_).getMember("get_container_client").getACall()
+  or
+  result = getBlobClient(_).getMember("_get_container_client").getACall()
 }

-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."
+API::Node getContainerClient(boolean isSource) {
+  isSource = false and
+  result = getTransitionToContainerClient().getReturn()
+  or
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("ContainerClient")
+        .getReturn()
+  or
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("ContainerClient")
+        .getMember(["from_connection_string", "from_container_url"])
+        .getReturn()
+}
+
+API::CallNode getTransitionToBlobClient() {
+  result = [getBlobServiceClient(_), getContainerClient(_)].getMember("get_blob_client").getACall()
+}
+
+API::Node getBlobClient(boolean isSource) {
+  isSource = false and
+  result = getTransitionToBlobClient().getReturn()
+  or
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("BlobClient")
+        .getReturn()
+  or
+  isSource = true and
+  result =
+    API::moduleImport("azure")
+        .getMember("storage")
+        .getMember("blob")
+        .getMember("BlobClient")
+        .getMember(["from_connection_string", "from_blob_url"])
+        .getReturn()
+}
+
+API::Node anyClient(boolean isSource) {
+  result in [getBlobServiceClient(isSource), getContainerClient(isSource), getBlobClient(isSource)]
+}
+
+newtype TAzureFlowState =
+  MkUsesV1Encryption() or
+  MkUsesNoEncryption()
+
+module AzureBlobClientConfig implements DataFlow::StateConfigSig {
+  class FlowState = TAzureFlowState;
+
+  predicate isSource(DataFlow::Node node, FlowState state) {
+    state = MkUsesNoEncryption() and
+    node = anyClient(true).asSource()
+  }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) {
+    exists(state) and
+    exists(DataFlow::AttrWrite attr |
+      node = anyClient(_).getAValueReachableFromSource() and
+      attr.accesses(node, "encryption_version") and
+      attr.getValue().asExpr().(StrConst).getText() in ["'2.0'", "2.0"]
+    )
+    or
+    // small optimization to block flow with no encryption out of the post-update node
+    // for the attribute assignment.
+    isAdditionalFlowStep(_, MkUsesNoEncryption(), node, MkUsesV1Encryption()) and
+    state = MkUsesNoEncryption()
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(DataFlow::MethodCallNode call |
+      call in [getTransitionToContainerClient(), getTransitionToBlobClient()] and
+      node1 = call.getObject() and
+      node2 = call
+    )
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+  ) {
+    node1 = node2.(DataFlow::PostUpdateNode).getPreUpdateNode() and
+    state1 = MkUsesNoEncryption() and
+    state2 = MkUsesV1Encryption() and
+    exists(DataFlow::AttrWrite attr |
+      node1 = anyClient(_).getAValueReachableFromSource() and
+      attr.accesses(node1, ["key_encryption_key", "key_resolver_function"])
+    )
+  }
+
+  predicate isSink(DataFlow::Node node, FlowState state) {
+    state = MkUsesV1Encryption() and
+    exists(DataFlow::MethodCallNode call |
+      call = getBlobClient(_).getMember("upload_blob").getACall() and
+      node = call.getObject()
+    )
+  }
+}
+
+module AzureBlobClient = DataFlow::GlobalWithState<AzureBlobClientConfig>;
+
+import AzureBlobClient::PathGraph
+
+from AzureBlobClient::PathNode source, AzureBlobClient::PathNode sink
+where AzureBlobClient::flowPath(source, sink)
+select sink, source, sink, "Unsafe usage of v1 version of Azure Storage client-side encryption"
--- a/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/UnsafeUsageOfClientSideEncryptionVersion.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/UnsafeUsageOfClientSideEncryptionVersion.expected
@@ -0,0 +1,66 @@
+edges
+| test.py:0:0:0:0 | ModuleVariableNode for test.BSC | test.py:7:19:7:21 | ControlFlowNode for BSC |
+| test.py:0:0:0:0 | ModuleVariableNode for test.BSC | test.py:35:19:35:21 | ControlFlowNode for BSC |
+| test.py:0:0:0:0 | ModuleVariableNode for test.BSC | test.py:66:19:66:21 | ControlFlowNode for BSC |
+| test.py:3:1:3:3 | GSSA Variable BSC | test.py:0:0:0:0 | ModuleVariableNode for test.BSC |
+| test.py:3:7:3:51 | ControlFlowNode for Attribute() | test.py:3:1:3:3 | GSSA Variable BSC |
+| test.py:7:19:7:21 | ControlFlowNode for BSC | test.py:8:5:8:15 | ControlFlowNode for blob_client |
+| test.py:8:5:8:15 | ControlFlowNode for blob_client | test.py:9:5:9:15 | ControlFlowNode for blob_client |
+| test.py:9:5:9:15 | ControlFlowNode for blob_client | test.py:9:5:9:15 | [post] ControlFlowNode for blob_client |
+| test.py:9:5:9:15 | [post] ControlFlowNode for blob_client | test.py:11:9:11:19 | ControlFlowNode for blob_client |
+| test.py:15:27:15:71 | ControlFlowNode for Attribute() | test.py:16:5:16:23 | ControlFlowNode for blob_service_client |
+| test.py:16:5:16:23 | ControlFlowNode for blob_service_client | test.py:17:5:17:23 | ControlFlowNode for blob_service_client |
+| test.py:17:5:17:23 | ControlFlowNode for blob_service_client | test.py:17:5:17:23 | [post] ControlFlowNode for blob_service_client |
+| test.py:17:5:17:23 | [post] ControlFlowNode for blob_service_client | test.py:21:9:21:19 | ControlFlowNode for blob_client |
+| test.py:25:24:25:66 | ControlFlowNode for Attribute() | test.py:26:5:26:20 | ControlFlowNode for container_client |
+| test.py:26:5:26:20 | ControlFlowNode for container_client | test.py:27:5:27:20 | ControlFlowNode for container_client |
+| test.py:27:5:27:20 | ControlFlowNode for container_client | test.py:27:5:27:20 | [post] ControlFlowNode for container_client |
+| test.py:27:5:27:20 | [post] ControlFlowNode for container_client | test.py:31:9:31:19 | ControlFlowNode for blob_client |
+| test.py:35:19:35:21 | ControlFlowNode for BSC | test.py:36:5:36:15 | ControlFlowNode for blob_client |
+| test.py:36:5:36:15 | ControlFlowNode for blob_client | test.py:37:5:37:15 | ControlFlowNode for blob_client |
+| test.py:37:5:37:15 | ControlFlowNode for blob_client | test.py:37:5:37:15 | [post] ControlFlowNode for blob_client |
+| test.py:37:5:37:15 | [post] ControlFlowNode for blob_client | test.py:43:9:43:19 | ControlFlowNode for blob_client |
+| test.py:66:19:66:21 | ControlFlowNode for BSC | test.py:67:5:67:15 | ControlFlowNode for blob_client |
+| test.py:67:5:67:15 | ControlFlowNode for blob_client | test.py:68:5:68:15 | ControlFlowNode for blob_client |
+| test.py:68:5:68:15 | ControlFlowNode for blob_client | test.py:68:5:68:15 | [post] ControlFlowNode for blob_client |
+| test.py:68:5:68:15 | [post] ControlFlowNode for blob_client | test.py:69:12:69:22 | ControlFlowNode for blob_client |
+| test.py:69:12:69:22 | ControlFlowNode for blob_client | test.py:73:10:73:33 | ControlFlowNode for get_unsafe_blob_client() |
+| test.py:73:10:73:33 | ControlFlowNode for get_unsafe_blob_client() | test.py:75:9:75:10 | ControlFlowNode for bc |
+nodes
+| test.py:0:0:0:0 | ModuleVariableNode for test.BSC | semmle.label | ModuleVariableNode for test.BSC |
+| test.py:3:1:3:3 | GSSA Variable BSC | semmle.label | GSSA Variable BSC |
+| test.py:3:7:3:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:7:19:7:21 | ControlFlowNode for BSC | semmle.label | ControlFlowNode for BSC |
+| test.py:8:5:8:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:9:5:9:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:9:5:9:15 | [post] ControlFlowNode for blob_client | semmle.label | [post] ControlFlowNode for blob_client |
+| test.py:11:9:11:19 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:15:27:15:71 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:16:5:16:23 | ControlFlowNode for blob_service_client | semmle.label | ControlFlowNode for blob_service_client |
+| test.py:17:5:17:23 | ControlFlowNode for blob_service_client | semmle.label | ControlFlowNode for blob_service_client |
+| test.py:17:5:17:23 | [post] ControlFlowNode for blob_service_client | semmle.label | [post] ControlFlowNode for blob_service_client |
+| test.py:21:9:21:19 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:25:24:25:66 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:26:5:26:20 | ControlFlowNode for container_client | semmle.label | ControlFlowNode for container_client |
+| test.py:27:5:27:20 | ControlFlowNode for container_client | semmle.label | ControlFlowNode for container_client |
+| test.py:27:5:27:20 | [post] ControlFlowNode for container_client | semmle.label | [post] ControlFlowNode for container_client |
+| test.py:31:9:31:19 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:35:19:35:21 | ControlFlowNode for BSC | semmle.label | ControlFlowNode for BSC |
+| test.py:36:5:36:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:37:5:37:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:37:5:37:15 | [post] ControlFlowNode for blob_client | semmle.label | [post] ControlFlowNode for blob_client |
+| test.py:43:9:43:19 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:66:19:66:21 | ControlFlowNode for BSC | semmle.label | ControlFlowNode for BSC |
+| test.py:67:5:67:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:68:5:68:15 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:68:5:68:15 | [post] ControlFlowNode for blob_client | semmle.label | [post] ControlFlowNode for blob_client |
+| test.py:69:12:69:22 | ControlFlowNode for blob_client | semmle.label | ControlFlowNode for blob_client |
+| test.py:73:10:73:33 | ControlFlowNode for get_unsafe_blob_client() | semmle.label | ControlFlowNode for get_unsafe_blob_client() |
+| test.py:75:9:75:10 | ControlFlowNode for bc | semmle.label | ControlFlowNode for bc |
+subpaths
+#select
+| test.py:11:9:11:19 | ControlFlowNode for blob_client | test.py:3:7:3:51 | ControlFlowNode for Attribute() | test.py:11:9:11:19 | ControlFlowNode for blob_client | Unsafe usage of v1 version of Azure Storage client-side encryption |
+| test.py:21:9:21:19 | ControlFlowNode for blob_client | test.py:15:27:15:71 | ControlFlowNode for Attribute() | test.py:21:9:21:19 | ControlFlowNode for blob_client | Unsafe usage of v1 version of Azure Storage client-side encryption |
+| test.py:31:9:31:19 | ControlFlowNode for blob_client | test.py:25:24:25:66 | ControlFlowNode for Attribute() | test.py:31:9:31:19 | ControlFlowNode for blob_client | Unsafe usage of v1 version of Azure Storage client-side encryption |
+| test.py:43:9:43:19 | ControlFlowNode for blob_client | test.py:3:7:3:51 | ControlFlowNode for Attribute() | test.py:43:9:43:19 | ControlFlowNode for blob_client | Unsafe usage of v1 version of Azure Storage client-side encryption |
+| test.py:75:9:75:10 | ControlFlowNode for bc | test.py:3:7:3:51 | ControlFlowNode for Attribute() | test.py:75:9:75:10 | ControlFlowNode for bc | Unsafe usage of v1 version of Azure Storage client-side encryption |
--- a/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/UnsafeUsageOfClientSideEncryptionVersion.qlref
+++ b/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/UnsafeUsageOfClientSideEncryptionVersion.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql
--- a/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/test.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-327-UnsafeUsageOfClientSideEncryptionVersion/test.py
@@ -0,0 +1,89 @@
+from azure.storage.blob import BlobServiceClient, ContainerClient, BlobClient
+
+BSC = BlobServiceClient.from_connection_string(...)
+
+def unsafe():
+    # does not set encryption_version to 2.0, default is unsafe
+    blob_client = BSC.get_blob_client(...)
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream) # BAD
+
+
+def unsafe_setting_on_blob_service_client():
+    blob_service_client = BlobServiceClient.from_connection_string(...)
+    blob_service_client.require_encryption = True
+    blob_service_client.key_encryption_key = ...
+
+    blob_client = blob_service_client.get_blob_client(...)
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream)
+
+
+def unsafe_setting_on_container_client():
+    container_client = ContainerClient.from_connection_string(...)
+    container_client.require_encryption = True
+    container_client.key_encryption_key = ...
+
+    blob_client = container_client.get_blob_client(...)
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream)
+
+
+def potentially_unsafe(use_new_version=False):
+    blob_client = BSC.get_blob_client(...)
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+
+    if use_new_version:
+        blob_client.encryption_version = '2.0'
+
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream) # BAD
+
+
+def safe():
+    blob_client = BSC.get_blob_client(...)
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+    # GOOD: Must use `encryption_version` set to `2.0`
+    blob_client.encryption_version = '2.0'
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream) # OK
+
+
+def safe_different_order():
+    blob_client: BlobClient = BSC.get_blob_client(...)
+    blob_client.encryption_version = '2.0'
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        blob_client.upload_blob(stream) # OK
+
+
+def get_unsafe_blob_client():
+    blob_client = BSC.get_blob_client(...)
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+    return blob_client
+
+
+def unsafe_with_calls():
+    bc = get_unsafe_blob_client()
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        bc.upload_blob(stream) # BAD
+
+
+def get_safe_blob_client():
+    blob_client = BSC.get_blob_client(...)
+    blob_client.require_encryption = True
+    blob_client.key_encryption_key = ...
+    blob_client.encryption_version = '2.0'
+    return blob_client
+
+
+def safe_with_calls():
+    bc = get_safe_blob_client()
+    with open("decryptedcontentfile.txt", "rb") as stream:
+        bc.upload_blob(stream) # OK
				`@@ -0,0 +1 @@`
				`experimental/Security/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql`