Repository navigation
feat: judge SSRF flows on the destination their URL's constant text fixes - #166
Merged
Merged
Conversation
The ssrf rule now also reads how the URL of each flow is built, so its manifest requires the SSA form and the module's strings.
…ixes A fixed host makes a flow a hotspot; a fixed host and path with redirects disabled refutes it. Only + and f-strings are read, over string literals and module-level strings bound once. The rule refines its own verdict through TaintDetector.judge, so the advisory correlation keeps the refutation's.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ssrfnow judges each flow on how+and f-strings build its URL. A fixed host is treated as a limited proof, not as a proof that there is no SSRF. Everything the analysis does not prove stays reported.Closes #165.
scheme://hostand ends the authority with/,?or#before any input:"https://api.example.com/users/" + uid,f"https://api.example.com/users/{uid}",API + "users/" + uidallow_redirects=False,follow_redirects=FalseTypeError, so either way no redirect is followed.**"https:/"+"/" + path); a constant leaving it open (no/after the host, a variable port, no scheme);urljoin;%;format; a URL built in a helper function.A module-level name counts as known text only when the module binds it once, to a string literal, and never rebinds it:
fortarget in the module body;global;Design
abstract/strings.py.leading_text(value, defs, strings)gives the constant text a value starts with, reading only+and f-strings.ModuleStringsAnalysis(abstract.module_strings) gives the module-level strings bound once. It counts everyStoreLocalof the module body, everyfortarget (ForNext) and everySetGlobal. A module that does not lower entirely has none.TaintDetector.judge(ctx, function, flow, verdict). A new hook, which by default returns the refutation's verdict.ssrfoverrides it, so only this rule's verdict changes. The advisory correlation reads the same flow, and it keeps the refutation's verdict; a test checks that theexploitable-vulnerabilityof a fixed-host call is still a vulnerability. Open redirects and the advisories that need the host are out of scope (Precision: an SSRF flow into a URL whose constant text fixes the host is a hotspot #165).ssrfmanifest now requiresir.ssaandabstract.module_strings.test_shipped_plugins_load_with_their_manifestsstates it, instead of listingssrfamong the rules requiring flows and refutation only.Corpus
The regression snapshots, which record each verdict, are unchanged. No
ssrffinding is refuted or downgraded:app/ssrf.py:12requests.get(url), withurl: str = Query(...)introduction/views.py:963url = request.POST["url"]app.py:60requests.get(request.args["url"])httpie/client.py:114requests_session.send(prepared_request)exploit_scripts/stored_xss_profile.py:24and:28f"{BASE_URL}/profile/{user_id}", withBASE_URLimported from_commonIn the last case,
_common.BASE_URLis"http://127.0.0.1:8000". Even read, it would only give a hotspot: the fixed host is internal, and the input chooses the path.Verification
tests/test_ssrf_destinations.py, 25 tests: the six variants, unknown and incomplete bases, module strings bound more than once (a second literal,global, afortarget, a definition), redirects (refuted, followed, disabled with the path chosen, unpacked options), the evidence text, a helper function, and the advisory verdict. The 11 tests expecting a hotspot or a refutation failed before the change.pytest: 1220 passed.pytest -m regression: 19 passed, unchanged.ruff checkandmypy: clean.