C++: Use pyrameterized modules for TaintTracking #1757
Conversation
| * 100` since we consider a single bit of information to be too little. | ||
| */ | ||
| module TaintTracking2 { | ||
| import semmle.code.cpp.dataflow.internal.TaintTrackingUtil |
There was a problem hiding this comment.
Shouldn't this import happen in dataflow2/TaintTrackingImpl.qll (and same for TaintTracking.qll)?
There was a problem hiding this comment.
It probably should. That would be consistent with how we do it for DataFlow. I'll change that.
jbj
force-pushed
the
pyrameterized-taint
branch
from
9085efe to
d2eea3e
Compare
|
I simplified this PR vastly after talking to @aschackmull. I dropped the commit that changed the data-flow libraries, so this PR now only changes the C/C++ taint tracking library. That means fewer code changes and fewer confusing indirections. |
| @@ -0,0 +1,2 @@ | |||
| import semmle.code.cpp.dataflow.DataFlow as Private | |||
There was a problem hiding this comment.
Should be dataflow2, right?
|
Fixing the @aschackmull, are you happy enough with these changes that I should apply the same restructuring to Java (and make the taint tracking implementation files identical across languages)? |
Yes. |
This explanation, taken from C/C++, was not correct for Java.
To keep the code changes minimal, and to keep the implementation similar
to C++ and Java, the `TaintTracking{Public,Private}` files are now
imported together through `TaintTrackingUtil`. This has the side effect
of exposing `localAdditionalTaintStep`. The corresponding predicate for
Java was already exposed.
jbj
force-pushed
the
pyrameterized-taint
branch
from
e9c7f06 to
bc702de
Compare
|
I now also made the change for C# and added change notes. I rebased the PR to avoid a change note conflict. |
csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingParameter.qll
Show resolved
Hide resolved
3 participants
This is a minimal PR to demonstrate how taint tracking can be pyrameterized such that each copy of the taint-tracking configuration class is byte-for-byte identical. It also switches the C++ interface from using
TaintTracking::Configuration2to usingTaintTracking2::Configurationlike C# does it.The commits should make sense individually: the first commit rearranges the data flow library, and the second commit makes the actual change to C++ taint tracking.
I didn't go as far as unifying the taint-tracking configuration classes across languages. I can follow up with language-specific PRs for that if we merge this PR.