Java/C++/C#: Add support for BarrierGuards.#1718
Conversation
|
This PR also fixes #1416. |
| * characteristic predicate precisely specifying the guard, and override | ||
| * `checks` to specify what is being validated and in which branch. | ||
| * | ||
| * It is important that all extending classes in scope are disjoint. |
There was a problem hiding this comment.
@xiemaisi Did you know about this issue? It should affect JavaScript too, but we couldn't find a non-contrived example where it would make a difference.
There was a problem hiding this comment.
The worst we could come up with was something like this:
var.regexMatches(safePattern)
var.regexMatches(patternToFindBadStuff)
If the two BarrierGuards describing those two case were sufficiently loosely described in the charpreds, such that there might exist regex patterns that fell into both, then you'd get into trouble when using either as a sanitizer if both were in scope (say from some barrier guard library).
There was a problem hiding this comment.
That's a good point. I hadn't previously considered it, and it hasn't caused us any problems, but we should probably call it out in our API docs as well. Thanks for flagging it up.
| import PathsCommon | ||
| import DataFlow::PathGraph | ||
|
|
||
| class ContainsDotDotSanitizer extends DataFlow::BarrierGuard { |
There was a problem hiding this comment.
Why not extend MethodAccess?
There was a problem hiding this comment.
I did initially, but then you get a couple of methods like getEnclosingCallable() that are ambiguously inherited, which means that we'll have to add additional overrides to the class.
Now if only QL allowed us to declare membership of a class without affecting dispatch...
| /** Holds if this guard validates `e` upon evaluating to `branch`. */ | ||
| abstract predicate checks(Expr e, boolean branch); | ||
|
|
||
| /** Gets a node guarded by this. */ |
There was a problem hiding this comment.
this -> this guard? (Same for other languages.)
|
Perhaps add a release note for #1416, since this was a user-reported FP? |
aschackmull
force-pushed
the
java/barrierguard
branch
from
fab4491 to
c99d0e7
Compare
|
Added change note for the |
5 participants
This adds support for BarrierGuards similar to the JavaScript dataflow library. Currently the C# and C++ classes are just stubs, so BarrierGuards won't work there just yet.
This doesn't add any new functionality to dataflow, but makes it easier to specify barriers/sanitizers that arise from guards. In particular it is unnecessary to know about things like SSA variables or the concept of a guard controlling a basicblock.
As an example, the
TaintedPathquery has been supplied with a!var.contains("..")sanitizer.