Repository navigation
feat(code-injection): report import directories chosen by input - #256
Merged
Merged
Conversation
Part of #240: sys.path and site.addsitedir. getattr follows in its own change.
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.
Part 2 of #240, first change: the import path.
getattr(...)()follows in its own PR.Summary
New sinks:
sys.path.insert(second argument; the index is data),sys.path.append,sys.path.extendandsite.addsitedir(sitedir) areCODEsinks withselects. Only a check that limits the directory to constants proves them.How findings are reported:
sys.path.*→ hotspot,Untrusted import path: code runs only when a later import resolves to a module the attacker placed there.site.addsitedir→ vulnerability,Untrusted site directory: the import lines of the directory's.pthfiles run during the call. The evidence states the condition: exploitable when the directory holds files the attacker controls.Coverage is partial, as documented in
docs/usage.md: writes tosys.paththat are not calls (slice assignment,+=, rebinding) are not reported.Verification
tests/test_import_path_sinks.py, new: 23 tests; the 12 positive cases fail on main. They cover:No existing test changed.
Full suite green, apart from the local
test_packagingmetadata issue.ruff checkandmypyclean. Regression snapshots unchanged; no new finding on mitmproxy or healthchecks.