Java: Add models for Commons-Lang's RegExUtils class#5339
Java: Add models for Commons-Lang's RegExUtils class#5339aschackmull merged 4 commits intogithub:mainfrom
Conversation
| Pattern cleanPattern = Pattern.compile("clean"); | ||
| Pattern taintedPattern = Pattern.compile(taint()); | ||
|
|
||
| sink(RegExUtils.removeAll(taint(), cleanPattern)); // $hasTaintFlow=y |
There was a problem hiding this comment.
I'll do this once that PR lands
| @@ -0,0 +1,2 @@ | |||
| lgtm,codescanning | |||
| * Added models for Apache Commons-Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. | |||
There was a problem hiding this comment.
Really minor, but it looks like the project itself does not use a hyphen, see https://commons.apache.org/proper/commons-lang/
| * Added models for Apache Commons-Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. | |
| * Added models for Apache Commons Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. |
|
These steps seem like something that could plausibly also be sanitisers in the context of some queries (and depending on the exact pattern). Historically I don't think we've modelled these sort of steps as taint steps for that reason. @lcartey Looks like you requested these steps - any insights on this? |
I would prefer to model these as taint steps, and add them as sanitizers for specific queries where that makes sense. I think this makes an appropriate trade-off between false positives and false negatives. For example, I don't think we should consider any regex approach as a sanitizer for SQL injection. |
SGTM. We should make sure to give similar treatment to the jdk equivalents + |
|
@aschackmull what order do you want these things to happen? |
It's probably fine to merge this PR as-is first, but the other additions should probably be accompanied with suitable sanitisers to avoid an intermediate state where some queries yield a bunch of extra FPs. |
c6e4cdd to
189b221
Compare
No description provided.