CPP: Support crement operations in CWE-190 #105
Conversation
change-notes/1.18/analysis-cpp.md
Outdated
|
|
||
| | [User-controlled data in arithmetic expression] | More correct results | Crement operations are now understood as arithmetic operations in this query. | | ||
| | [Uncontrolled data in arithmetic expression] | More correct results | Crement operations are now understood as arithmetic operations in this query. | | ||
| | [Use of extreme values in arithmetic expression] | More correct results | Crement operations are now understood as arithmetic operations in this query. | |
There was a problem hiding this comment.
I don't think "Crement" is a standard enough term to be used in a change note. Better write it out as "increment/decrement".
| use = e.getAnOperand() and | ||
| exists(LocalScopeVariable v | use.getTarget() = v | | ||
| // overflow possible if large | ||
| (e instanceof AddExpr and not guardedLesser(e, varUse(v))) or | ||
| (e instanceof IncrementOperation and not guardedLesser(e, varUse(v)) and v.getType().getUnspecifiedType() instanceof IntegralType) or |
There was a problem hiding this comment.
If we're supporting x + 1 and x++ then why not x += 1? Same for subtraction.
There was a problem hiding this comment.
Done. FWIW I think some of these cases are naive, the right-hand-side of subtraction is ignored and most cases assume the other side of a binary arithmetic expression is (at least sometimes) positive. I'll make a JIRA ticket to look into improving this.
| (isMaxValue(expr) and cause = "max value") or | ||
| (isMinValue(expr) and cause = "min value") | ||
| (isMaxValue(expr) and cause = "overflow") or | ||
| (isMinValue(expr) and cause = "underflow") |
There was a problem hiding this comment.
I think the old strings match better what this predicate actually computes. Unless there's a good reason to change these strings, I suggest keeping them as they are and having both a string cause and a string effect in the query, matching them up in the or formulas. It looks strange now that cause and effect are conflated.
geoffw0
force-pushed
the
samate-crement
branch
from
06882f8 to
5d37c1a
Compare
geoffw0
force-pushed
the
samate-crement
branch
2 times, most recently
from
b6ccb9c to
0d63739
Compare
Kotlin: Add tests for `this`
New performance query: Transitive step in recursion.
…step New performance query: Transitive step in recursion.
Add CodeQL rule for Immutable actions, do not detect immutable actions in unpinned tag rule
Support crement operations in the CWE-190 queries. This fixes one of the 'regressions' we had when we changed from using Samate Juliet Test Suite version 1.2 to 1.3.
There's also a small improvement to the logic in
ArithmeticWithExtremeValues.qlso that it understands the direction of the potential over/under-flow compared to the extreme value.