Merge pull request #32 from RasmusWL/azure-blob-client

`py/azure-storage/unsafe-client-side-encryption-in-use` updates
2026-04-30 03:05:15 +02:00 · 2023-03-29 20:26:25 -07:00
parent 4ba1740c45 34cbaf10c2
commit 05137d0c54
4 changed files with 295 additions and 36 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,42 +12,145 @@
 */

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

-predicate isUnsafeClientSideAzureStorageEncryptionViaAttributes(Call call, AttrNode node) {
-  exists(
-    API::Node n, ControlFlowNode startingNode, Attribute attr, ControlFlowNode ctrlFlowNode,
-    Attribute attrUploadBlob, ControlFlowNode ctrlFlowNodeUploadBlob, string s1, string s2,
-    string s3
-  |
-    call.getAChildNode() = attrUploadBlob and
-    node = ctrlFlowNode
-  |
-    s1 in ["key_encryption_key", "key_resolver_function"] and
-    s2 in ["ContainerClient", "BlobClient", "BlobServiceClient"] and
-    s3 = "upload_blob" and
-    n = API::moduleImport("azure").getMember("storage").getMember("blob").getMember(s2).getAMember() and
-    startingNode = n.getACall().getReturn().getAValueReachableFromSource().asExpr().getAFlowNode() and
-    startingNode.strictlyReaches(ctrlFlowNode) and
-    attr.getAFlowNode() = ctrlFlowNode and
-    attr.getName() = s1 and
-    ctrlFlowNode.strictlyReaches(ctrlFlowNodeUploadBlob) and
-    attrUploadBlob.getAFlowNode() = ctrlFlowNodeUploadBlob and
-    attrUploadBlob.getName() = s3 and
-    not exists(
-      Attribute attrBarrier, ControlFlowNode ctrlFlowNodeBarrier, AssignStmt astmt2, StrConst uc
-    |
-      startingNode.strictlyReaches(ctrlFlowNodeBarrier) and
-      attrBarrier.getAFlowNode() = ctrlFlowNodeBarrier and
-      attrBarrier.getName() = "encryption_version" and
-      uc = astmt2.getValue() and
-      uc.getText() in ["'2.0'", "2.0"] and
-      astmt2.getATarget().getAChildNode*() = attrBarrier and
-      ctrlFlowNodeBarrier.strictlyReaches(ctrlFlowNodeUploadBlob)
-    )
-  )
+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()
 }

-from Call call, ControlFlowNode node
-where isUnsafeClientSideAzureStorageEncryptionViaAttributes(call, node)
-select node, "Unsafe usage of v1 version of Azure Storage client-side encryption."
+API::CallNode getTransitionToContainerClient() {
+  result = getBlobServiceClient(_).getMember("get_container_client").getACall()
+  or
+  result = getBlobClient(_).getMember("_get_container_client").getACall()
+}
+
+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 | ControlFlowNode for blob_client |
+| test.py:9:5:9:15 | 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 | ControlFlowNode for blob_service_client |
+| test.py:17:5:17:23 | 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 | ControlFlowNode for container_client |
+| test.py:27:5:27:20 | 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 | ControlFlowNode for blob_client |
+| test.py:37:5:37:15 | 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 | ControlFlowNode for blob_client |
+| test.py:68:5:68:15 | 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 | ControlFlowNode for blob_client | semmle.label | 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 | ControlFlowNode for blob_service_client | semmle.label | 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 | ControlFlowNode for container_client | semmle.label | 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 | ControlFlowNode for blob_client | semmle.label | 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 | ControlFlowNode for blob_client | semmle.label | 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`