Java: add ssrf models discovered with heuristics#12155
Conversation
94bee1b to
7510327
Compare
c57b8af to
15a1cbb
Compare
1f4e6e6 to
dd21ccc
Compare
85191b6 to
e85ae83
Compare
…he.http.client.methods.HttpRequestBase.setURI instead
…l; resolve conflicts
…c.client5.http.classic.methods
…sting apache-http-5 stubs
686de32 to
523feab
Compare
atorralba
left a comment
There was a problem hiding this comment.
I started this review as if this PR was normal library modelling work (i.e. looking for both incorrect models and missing models), but I don't think that's the correct way to look at it, since you're adding only things found through heuristics.
So, consider my review to only have looked into correctness, but not completeness. With that in mind, this LGTM.
Also, I used
majorAnalysissince this PR adds a lot of Apache HttpComponents version 5 models, but let me know if it should beminorAnalysisinstead.
The distinction is subtle in this case because this is a big, popular library, so I don't think it matters much, but I feel inclined to always consider modelling work as minorAnalysis (unless we model something really fundamental, like the full JDK or a good part of it, that is virtually going to affect every analysis). I usually see majorAnalysis reserved to deeper changes, like an overhaul of the dataflow library or similar "everyone is impacted" things.
Feel free to leave it as it is if you have a strong opinion though.
I don't have a strong opinion, so I've switched it to |
Description
org.apache.http.HttpRequest#setURIsince this model is a typo and is covered by the model fororg.apache.http.client.methods.HttpRequestBase#setURIinstead.io.netty.handler.codec.http.HttpRequest#setUrito a sink model instead.Consideration
majorAnalysissince this PR adds a lot of Apache HttpComponents version 5 models, but let me know if it should beminorAnalysisinstead.