C++: Fix false negatives for postfix crement expressions#2799
Conversation
|
I made a similar change in the original version of #1999, but based on feedback from @dbartol in #1999 (comment) I changed it so there's no instruction mapped to the result of an assignment/crement instruction in void context. I think the main motivation was to keep the size of the IR minimal. Can you work around this problem in |
Thanks for the heads up. It's an interesting read.
I've pushed a commit that does this now. Hopefully this is along the lines of what you suggested. |
|
Looks good. Is there a need to do something similar for |
For I'm not sure if any query benefits from propagating taint from an operand to an |
|
I've added the noncontroversial part of @jbj's comment (i.e., the I've left out adding |
|
I don't think flow to the result of |
…lse negatives reported by Geoffrey
This commit fixes an issue where a
PostfixIncrExprin a void context would not be assigned aDataFlow::Nodewhen switching toDefaultTaintTracking. i.e., the querynever returned any results. This meant that
adjustedSinkinDefaultTaintTrackingnever returned aPostfixIncrExpr.The fix is to always generate aThe fix is to add a case forCopyValueInstructioneven though thee++(ore--) expression happens in a void context.PostfixCrementOperationinadjustedSink. This recovers two false negatives inSecurity/CWE/CWE-190/semmle/tainted/IntegerOverflowTainted.expected: