-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Java: Refactor path injection sinks #12886
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
Java: Refactor path injection sinks #12886
Conversation
bbfd7af to
edca5a9
Compare
jcogs33
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming DCA is good.
5ecb5c3 to
9e469c9
Compare
jcogs33
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming that the CI check failures are unrelated to any changes in the PR. I don't recall seeing those failures yesterday.
They were caused by missing an import during a conflict resolution after the rebase. It's been fixed in 5bcf687. |
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,10,733,237,79,,9,,,24
+ Java Standard Library,``java.*``,10,734,232,74,,9,,,24
- Totals,,308,18947,2551,331,16,128,33,1,407
+ Totals,,308,18948,2546,326,16,128,33,1,407
- java.nio,49,,36,,,,,,,,,5,,,,,,,,,,,,,,,43,,,,,,,,,1,,,,,,,,,,,,,,36,
+ java.nio,44,,37,,,,,,,,,5,,,,,,,,,,,,,,,38,,,,,,,,,1,,,,,,,,,,,,,,37, |
|
I added the models-as-data migration part to a new PR #13225. I'll repurpose this PR to be only about the sink refactor part (converting path creation sinks into summaries). For the time being, I'm going to leave this as a draft. |
Deprecate and stop using PathCreation Path creation sinks are now summaries
5bcf687 to
2967967
Compare
2967967 to
2a14640
Compare
ZipSlip no longer needs to make this exclusion, since PathCreation arguments are no longer path-injection sinks
f54c30b to
8e105ec
Compare
8e105ec to
e2bf9ea
Compare
egregius313
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Now we consider path creation methods as summaries instead of sinks, and actual sinks are now proper file read/write operations. This is because, many times, path-like objects (
FileorPath) are created as part of user input validation/sanitization logic, so raising an alert that early led to many FPs. Now we follow the flow path until the actual filesystem IO operation happens, which should help with precision significantly.Significant alert changes in the affected queries are expected after these changes.