Skip to content

Commit 5ad42f8

Browse files
authored
Merge pull request #20563 from microsoft/azure_python_sdk_url_summary_upstream
Azure python sdk url summary upstream
2 parents b5e3168 + 8459eec commit 5ad42f8

File tree

7 files changed

+181
-0
lines changed

7 files changed

+181
-0
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added request forgery sink models for the Azure SDK.
5+
* Made it so that models-as-data sinks with the kind `request-forgery` contribute to the class `Http::Client::Request` which represents HTTP client requests.

python/ql/lib/semmle/python/Concepts.qll

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ private import semmle.python.security.internal.EncryptionKeySizes
1515
private import semmle.python.dataflow.new.SensitiveDataSources
1616
private import codeql.threatmodels.ThreatModels
1717
private import codeql.concepts.ConceptsShared
18+
private import semmle.python.ApiGraphs
19+
private import semmle.python.frameworks.data.ModelsAsData
1820

1921
private module ConceptsShared = ConceptsMake<Location, PythonDataFlow>;
2022

@@ -1656,8 +1658,35 @@ module Http {
16561658
}
16571659

16581660
import ConceptsShared::Http::Client as Client
1661+
16591662
// TODO: investigate whether we should treat responses to client requests as
16601663
// remote-flow-sources in general.
1664+
/**
1665+
* An HTTP request modeled from `request-forgery` sinks, modeled using MaD.
1666+
*/
1667+
class HttpClientRequestFromModel extends Http::Client::Request::Range instanceof API::CallNode {
1668+
DataFlow::Node urlArg;
1669+
1670+
HttpClientRequestFromModel() {
1671+
(
1672+
this.getArg(_) = urlArg
1673+
or
1674+
this.getArgByName(_) = urlArg
1675+
) and
1676+
ModelOutput::sinkNode(urlArg, "request-forgery")
1677+
}
1678+
1679+
override DataFlow::Node getAUrlPart() { result = urlArg }
1680+
1681+
override string getFramework() { result = "MaD" }
1682+
1683+
override predicate disablesCertificateValidation(
1684+
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
1685+
) {
1686+
// NOTE: if you need to define this, you have to special case it for every possible API in MaD
1687+
none()
1688+
}
1689+
}
16611690
}
16621691

16631692
/**
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/python-all
4+
extensible: sinkModel
5+
data:
6+
- ['azure.keyvault.certificates.CertificateClient!', 'Call.Argument[0,vault_url:]', 'request-forgery']
7+
- ['azure.keyvault.certificates.DeletedCertificate!', 'Call.Argument[recovery_id:]', 'request-forgery']
8+
- ['azure.keyvault.keys.KeyClient!', 'Call.Argument[0,vault_url:]', 'request-forgery']
9+
- ['azure.keyvault.secrets.SecretClient!', 'Call.Argument[0,vault_url:]', 'request-forgery']
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/python-all
4+
extensible: sinkModel
5+
data:
6+
- ['azure.storage.blob.BlobClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
7+
- ['azure.storage.blob.BlobClient', 'Member[append_block_from_url].Argument[0,copy_source_url:]', 'request-forgery']
8+
- ['azure.storage.blob.BlobClient', 'Member[get_page_range_diff_for_managed_disk].Argument[0,previous_snapshot_url:]', 'request-forgery']
9+
- ['azure.storage.blob.BlobClient', 'Member[stage_block_from_url].Argument[1,source_url:]', 'request-forgery']
10+
- ['azure.storage.blob.BlobClient', 'Member[start_copy_from_url].Argument[0,source_url:]', 'request-forgery']
11+
- ['azure.storage.blob.BlobClient', 'Member[upload_blob_from_url].Argument[0,source_url:]', 'request-forgery']
12+
- ['azure.storage.blob.BlobClient', 'Member[upload_pages_from_url].Argument[0,source_url:]', 'request-forgery']
13+
- ['azure.storage.blob.BlobClient!', 'Member[from_blob_url].Argument[0,blob_url:]', 'request-forgery']
14+
- ['azure.storage.blob.BlobServiceClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
15+
- ['azure.storage.blob.ContainerClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
16+
- ['azure.storage.blob.ContainerClient!', 'Member[from_container_url].Argument[0,container_url:]', 'request-forgery']
17+
- ['azure', 'Member[storage].Member[blob].Member[download_blob_from_url].Argument[0,blob_url:]', 'request-forgery']
18+
- ['azure', 'Member[storage].Member[blob].Member[upload_blob_to_url].Argument[0,blob_url:]', 'request-forgery']
19+
- ['azure.storage.filedatalake.DataLakeDirectoryClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
20+
- ['azure.storage.filedatalake.DataLakeFileClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
21+
- ['azure.storage.filedatalake.DataLakeServiceClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
22+
- ['azure.storage.filedatalake.FileSystemClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
23+
- ['azure.storage.fileshare.ShareClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
24+
- ['azure.storage.fileshare.ShareClient!', 'Member[from_share_url].Argument[0,share_url:]', 'request-forgery']
25+
- ['azure.storage.fileshare.ShareDirectoryClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
26+
- ['azure.storage.fileshare.ShareDirectoryClient!', 'Member[from_directory_url].Argument[0,directory_url:]', 'request-forgery']
27+
- ['azure.storage.fileshare.ShareFileClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
28+
- ['azure.storage.fileshare.ShareFileClient!', 'Member[from_file_url].Argument[0,file_url:]', 'request-forgery']
29+
- ['azure.storage.fileshare.ShareFileClient', 'Member[start_copy_from_url].Argument[0,source_url:]', 'request-forgery']
30+
- ['azure.storage.fileshare.ShareFileClient', 'Member[upload_range_from_url].Argument[0,source_url:]', 'request-forgery']
31+
- ['azure.storage.fileshare.ShareServiceClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
32+
- ['azure.storage.queue.QueueClient!', 'Call.Argument[0,account_url:]', 'request-forgery']
33+
- ['azure.storage.queue.QueueClient', 'Member[from_queue_url].Argument[0,queue_url:]', 'request-forgery']
34+
- ['azure.storage.queue.QueueServiceClient!', 'Call.Argument[0,account_url:]', 'request-forgery']

python/ql/test/query-tests/Security/CWE-918-ServerSideRequestForgery/FullServerSideRequestForgery.expected

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ edges
3535
| full_partial_test.py:75:5:75:7 | ControlFlowNode for url | full_partial_test.py:76:18:76:20 | ControlFlowNode for url | provenance | |
3636
| full_partial_test.py:78:5:78:7 | ControlFlowNode for url | full_partial_test.py:79:18:79:20 | ControlFlowNode for url | provenance | |
3737
| full_partial_test.py:81:5:81:7 | ControlFlowNode for url | full_partial_test.py:82:18:82:20 | ControlFlowNode for url | provenance | |
38+
| test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:7:19:7:25 | ControlFlowNode for request | provenance | |
39+
| test_azure_client.py:7:19:7:25 | ControlFlowNode for request | test_azure_client.py:10:18:10:24 | ControlFlowNode for request | provenance | |
40+
| test_azure_client.py:7:19:7:25 | ControlFlowNode for request | test_azure_client.py:11:19:11:25 | ControlFlowNode for request | provenance | |
41+
| test_azure_client.py:10:18:10:24 | ControlFlowNode for request | test_azure_client.py:11:5:11:15 | ControlFlowNode for user_input2 | provenance | AdditionalTaintStep |
42+
| test_azure_client.py:11:5:11:15 | ControlFlowNode for user_input2 | test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | provenance | |
43+
| test_azure_client.py:11:19:11:25 | ControlFlowNode for request | test_azure_client.py:11:5:11:15 | ControlFlowNode for user_input2 | provenance | AdditionalTaintStep |
44+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | test_azure_client.py:17:32:17:39 | ControlFlowNode for full_url | provenance | Sink:MaD:15 |
45+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | test_azure_client.py:19:39:19:46 | ControlFlowNode for full_url | provenance | Sink:MaD:38 |
46+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | test_azure_client.py:21:19:21:26 | ControlFlowNode for full_url | provenance | Sink:MaD:14 |
47+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | test_azure_client.py:23:58:23:65 | ControlFlowNode for full_url | provenance | Sink:MaD:26 |
48+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | test_azure_client.py:32:18:32:25 | ControlFlowNode for full_url | provenance | Sink:MaD:27 |
3849
| test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | test_http_client.py:1:26:1:32 | ControlFlowNode for request | provenance | |
3950
| test_http_client.py:1:26:1:32 | ControlFlowNode for request | test_http_client.py:9:19:9:25 | ControlFlowNode for request | provenance | |
4051
| test_http_client.py:1:26:1:32 | ControlFlowNode for request | test_http_client.py:10:19:10:25 | ControlFlowNode for request | provenance | |
@@ -89,6 +100,17 @@ nodes
89100
| full_partial_test.py:79:18:79:20 | ControlFlowNode for url | semmle.label | ControlFlowNode for url |
90101
| full_partial_test.py:81:5:81:7 | ControlFlowNode for url | semmle.label | ControlFlowNode for url |
91102
| full_partial_test.py:82:18:82:20 | ControlFlowNode for url | semmle.label | ControlFlowNode for url |
103+
| test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
104+
| test_azure_client.py:7:19:7:25 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
105+
| test_azure_client.py:10:18:10:24 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
106+
| test_azure_client.py:11:5:11:15 | ControlFlowNode for user_input2 | semmle.label | ControlFlowNode for user_input2 |
107+
| test_azure_client.py:11:19:11:25 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
108+
| test_azure_client.py:14:5:14:12 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
109+
| test_azure_client.py:17:32:17:39 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
110+
| test_azure_client.py:19:39:19:46 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
111+
| test_azure_client.py:21:19:21:26 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
112+
| test_azure_client.py:23:58:23:65 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
113+
| test_azure_client.py:32:18:32:25 | ControlFlowNode for full_url | semmle.label | ControlFlowNode for full_url |
92114
| test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
93115
| test_http_client.py:1:26:1:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
94116
| test_http_client.py:9:5:9:15 | ControlFlowNode for unsafe_host | semmle.label | ControlFlowNode for unsafe_host |
@@ -122,6 +144,11 @@ subpaths
122144
| full_partial_test.py:76:5:76:21 | ControlFlowNode for Attribute() | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | full_partial_test.py:76:18:76:20 | ControlFlowNode for url | The full URL of this request depends on a $@. | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | user-provided value |
123145
| full_partial_test.py:79:5:79:21 | ControlFlowNode for Attribute() | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | full_partial_test.py:79:18:79:20 | ControlFlowNode for url | The full URL of this request depends on a $@. | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | user-provided value |
124146
| full_partial_test.py:82:5:82:21 | ControlFlowNode for Attribute() | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | full_partial_test.py:82:18:82:20 | ControlFlowNode for url | The full URL of this request depends on a $@. | full_partial_test.py:1:19:1:25 | ControlFlowNode for ImportMember | user-provided value |
147+
| test_azure_client.py:17:9:17:63 | ControlFlowNode for SecretClient() | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:17:32:17:39 | ControlFlowNode for full_url | The full URL of this request depends on a $@. | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |
148+
| test_azure_client.py:19:9:19:47 | ControlFlowNode for Attribute() | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:19:39:19:46 | ControlFlowNode for full_url | The full URL of this request depends on a $@. | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |
149+
| test_azure_client.py:21:9:21:39 | ControlFlowNode for KeyClient() | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:21:19:21:26 | ControlFlowNode for full_url | The full URL of this request depends on a $@. | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |
150+
| test_azure_client.py:23:9:23:89 | ControlFlowNode for Attribute() | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:23:58:23:65 | ControlFlowNode for full_url | The full URL of this request depends on a $@. | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |
151+
| test_azure_client.py:31:5:36:5 | ControlFlowNode for download_blob_from_url() | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | test_azure_client.py:32:18:32:25 | ControlFlowNode for full_url | The full URL of this request depends on a $@. | test_azure_client.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |
125152
| test_http_client.py:14:5:14:36 | ControlFlowNode for Attribute() | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | test_http_client.py:13:27:13:37 | ControlFlowNode for unsafe_host | The full URL of this request depends on a $@. | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
126153
| test_http_client.py:14:5:14:36 | ControlFlowNode for Attribute() | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | test_http_client.py:14:25:14:35 | ControlFlowNode for unsafe_path | The full URL of this request depends on a $@. | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
127154
| test_http_client.py:19:5:19:36 | ControlFlowNode for Attribute() | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | test_http_client.py:18:27:18:37 | ControlFlowNode for unsafe_host | The full URL of this request depends on a $@. | test_http_client.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |

0 commit comments

Comments
 (0)