Currently there is support for just adding taint on ProgramState, this would allow to mark symbols as non-tainted.
I don't remember why the 'tainted' methods were added to ProgramState in the first place, but it doesn't seem quite right. Taint can easily be modeled as a set of APIs that modify and produce new ProgramStates, but I don't see why it should be part of ProgramState itself.
In general I'm not a fan of the taint methods being directly on ProgramState, but this extends the current pattern. We could move the APIs elsewhere later.
The parameters from the two lines should line up.
This seems reasonable enough. It mostly mirrors the other taint operations we have, and there is noticeably a fair amount of copy-paste here that I wonder could be better factored. removeTaint looks pretty much identical to addTaint except that it uses the remove function on the TaintMap. I'm particularly concerned about the copy-paste for the first removeTaint, as that is a non-trivial amount of logic. Could this case possibly be refactored a bit with addTaint?
Yes, I agree with you Ted. We can leave as it is just for now, and then I can work on that patch. May I assign you as subscriber when submitting it?
Yes, sure! Let me upload a new patch for this.
Basically, I moved the logic of AddTaint out of the method, and placed it on a generic method which allows to add or remove over the map based on a parameter value. All this taint logic will be placed on a external API in a future.
Can we add documentation comments for these? Seems like generally useful utility methods. We could also probably just make these public.
Is this even needed? I think Environment::getSVal() already handles parenthesis and other ignored expressions. This looks like dead code.
This can probably just be an inline method in ProgramState.h, that just forwards to getSVal(S, LCtx).getAsSymbol().
Alternatively, if this is only called once, we don't need to add a method at all, since it is just a one liner.
This looks fishy. 'addTaint' returns a new state, but then the return value is ignored, and 'this' is returned.
This is just a one-liner. Do we really need this method at all? It is only called once.
This looks fishy. 'removeTaint' returns a new state, but then the return value is ignored. 'ProgramState' values are immutable, so this method appears to do nothing.
Actually, I'm wondering if we really need to add these at all. They are just one liners that easily could be written where they are used.
Right. Removing these methods, and adding the one-liners directly where they are used.
Yes, you are right. It is not needed since it is handle by ignoreTransparentExprs method in Environment module. I will not add this method at all.
Yes, it does.. I will return at the time the last addTaint is invoked.
We don't. I will add the one-liner directly where it is used.
Yes, you are right. I will return at the time the last addTaint is invoked.