-
Notifications
You must be signed in to change notification settings - Fork 2k
Azure python sdk url summary upstream #20563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
47fac88
d27d4fd
704e296
341f553
5ca9ff2
fab96d9
d790c6d
acddb2c
a660eab
26b8a39
7ddfa80
cd73dcf
0a88425
ac1987f
8459eec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Added `ssrf` MaD for the azure SDK | ||
| * Added MaD `ssrf` to `Http::Client::Request` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| extensions: | ||
| - addsTo: | ||
| pack: codeql/python-all | ||
| extensible: sinkModel | ||
| data: | ||
| - ['azure.keyvault.certificates.CertificateClient!', 'Call.Argument[0,vault_url:]', 'ssrf'] | ||
| - ['azure.keyvault.certificates.DeletedCertificate!', 'Call.Argument[recovery_id:]', 'ssrf'] | ||
| - ['azure.keyvault.keys.KeyClient!', 'Call.Argument[0,vault_url:]', 'ssrf'] | ||
| - ['azure.keyvault.secrets.SecretClient!', 'Call.Argument[0,vault_url:]', 'ssrf'] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| extensions: | ||
| - addsTo: | ||
| pack: codeql/python-all | ||
| extensible: sinkModel | ||
| data: | ||
| - ['azure.storage.blob.BlobClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[append_block_from_url].Argument[0,copy_source_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[get_page_range_diff_for_managed_disk].Argument[0,previous_snapshot_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[stage_block_from_url].Argument[1,source_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[start_copy_from_url].Argument[0,source_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[upload_blob_from_url].Argument[0,source_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient', 'Member[upload_pages_from_url].Argument[0,source_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobClient!', 'Member[from_blob_url].Argument[0,blob_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.BlobServiceClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.ContainerClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.blob.ContainerClient!', 'Member[from_container_url].Argument[0,container_url:]', 'ssrf'] | ||
| - ['azure', 'Member[storage].Member[blob].Member[download_blob_from_url].Argument[0,blob_url:]', 'ssrf'] | ||
| - ['azure', 'Member[storage].Member[blob].Member[upload_blob_to_url].Argument[0,blob_url:]', 'ssrf'] | ||
| - ['azure.storage.filedatalake.DataLakeDirectoryClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.filedatalake.DataLakeFileClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.filedatalake.DataLakeServiceClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.filedatalake.FileSystemClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareClient!', 'Member[from_share_url].Argument[0,share_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareDirectoryClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareDirectoryClient!', 'Member[from_directory_url].Argument[0,directory_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareFileClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareFileClient!', 'Member[from_file_url].Argument[0,file_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareFileClient', 'Member[start_copy_from_url].Argument[0,source_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareFileClient', 'Member[upload_range_from_url].Argument[0,source_url:]', 'ssrf'] | ||
| - ['azure.storage.fileshare.ShareServiceClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.queue.QueueClient!', 'Call.Argument[0,account_url:]', 'ssrf'] | ||
| - ['azure.storage.queue.QueueClient', 'Member[from_queue_url].Argument[0,queue_url:]', 'ssrf'] | ||
| - ['azure.storage.queue.QueueServiceClient!', 'Call.Argument[0,account_url:]', 'ssrf'] |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file doesn't really match the way the rest of the repo is structured. Normally this kind of code would go in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (And I would name the class
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, this is my only remaining issue. There is also the philosophical question of whether all SSRF sinks really are client requests. If we had MaD for concepts, we could do this the other way round: Make all these models be client requests (with appropriate frameworks and certificate validation) and then they would be imported into the query automatically (and the query would decide that all client requests are SSRF sinks). But for now, following the suggestion with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've just pushed a change for this. The class was moved and renamed as suggested. I did remove it from a module though. Let me know how this looks. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| private import python | ||
| private import semmle.python.Concepts | ||
| private import semmle.python.ApiGraphs | ||
| private import semmle.python.frameworks.data.ModelsAsData | ||
|
|
||
| /** | ||
| * INTERNAL: Do not use. | ||
| * | ||
| * Sets up SSRF sinks as Http::Client::Request | ||
| */ | ||
| module SSRFMaDModel { | ||
|
|
||
| class SSRFSink extends Http::Client::Request::Range instanceof API::CallNode { | ||
|
|
||
| DataFlow::Node urlArg; | ||
|
|
||
| SSRFSink() { | ||
| ( | ||
| this.getArg(_) = urlArg | ||
| or | ||
| this.getArgByName(_) = urlArg | ||
| ) and | ||
| urlArg = ModelOutput::getASinkNode("ssrf").asSink() | ||
|
bdrodes marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| override DataFlow::Node getAUrlPart() { result = urlArg } | ||
|
|
||
| override string getFramework() { result = "MaD" } | ||
|
|
||
| override predicate disablesCertificateValidation( | ||
| DataFlow::Node disablingNode, DataFlow::Node argumentOrigin | ||
| ) { | ||
| // NOTE: if you need to define this, you have to special case it for every possible API in MaD | ||
| none() | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.