Visitar URL original
Azure python sdk url summary upstream by bdrodes · Pull Request #20563 · github/codeql · GitHub
Skip to content

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.

Sign up for GitHub

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

Merged
yoff merged 15 commits into from
Feb 9, 2026
Merged

Azure python sdk url summary upstream #20563

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
47fac88
Azure SDK models for SSRF analysis.
bdrodes Sep 26, 2025
d27d4fd
Updating comments.
bdrodes Sep 30, 2025
704e296
Adding azure sdk test cases and updated test expected file.
bdrodes Sep 30, 2025
341f553
Added change logs.
bdrodes Sep 30, 2025
5ca9ff2
Update python/ql/lib/semmle/python/frameworks/SSRFSink.qll
bdrodes Sep 30, 2025
fab96d9
Update python/ql/test/query-tests/Security/CWE-918-ServerSideRequestF…
bdrodes Sep 30, 2025
d790c6d
Update python/ql/test/query-tests/Security/CWE-918-ServerSideRequestF…
bdrodes Sep 30, 2025
acddb2c
Moved change log to correct location.
bdrodes Sep 30, 2025
a660eab
Adding docs.
bdrodes Sep 30, 2025
26b8a39
Adjusting acryonym for SSRF for casing standards.
bdrodes Sep 30, 2025
7ddfa80
Merge branch 'main' into azure_python_sdk_url_summary_upstream
bdrodes Feb 2, 2026
cd73dcf
Merge branch 'main' into azure_python_sdk_url_summary_upstream
bdrodes Feb 4, 2026
0a88425
Python: Altering SSRF MaD to use 'request-forgery' tag. Update to tes…
bdrodes Feb 4, 2026
ac1987f
Update python/ql/lib/change-notes/2025-09-30-azure_ssrf_models.md
bdrodes Feb 5, 2026
8459eec
Moving the SsrfSink concept into Concepts.qll, and renaming to HttpCl…
bdrodes Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updating comments.
  • Loading branch information
@bdrodes
bdrodes committed Sep 30, 2025
commit d27d4fdb2713470badceb7bbc81710560a09e79c
7 changes: 2 additions & 5 deletions python/ql/lib/semmle/python/frameworks/SSRFSink.qll
View file
Open in desktop

@owen-mc owen-mc Feb 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll, but then it would only affect just the two SSRF queries. Since these models are principally adding to Http::Client::Request::Range, and only indirectly becoming SSRF sinks, it might make more sense to put them somewhere like python/ql/lib/semmle/python/Concepts.qll, maybe just below line 1658 import ConceptsShared::Http::Client as Client.

@owen-mc owen-mc Feb 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And I would name the class HttpClientRequestFromModel.)

@yoff yoff Feb 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 HttpClientRequestFromModel would be ok.

@bdrodes bdrodes Feb 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Up @@ -23,15 +23,12 @@ module SSRFMaDModel {

override DataFlow::Node getAUrlPart() { result = urlArg }

override string getFramework() {
// TOOD: how to get type of this node?
result = "MaD"
}
override string getFramework() { result = "MaD" }

override predicate disablesCertificateValidation(
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
) {
// TODO: if you need to define this, you have to special case it for every possible API in MaD
// NOTE: if you need to define this, you have to special case it for every possible API in MaD
none()
}
}
Expand Down