mirror of
https://github.com/github/codeql.git
synced 2026-03-24 00:16:49 +01:00
C++: Move cpp/unsigned-difference-expression-compared-zero out of experimental.
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
unsigned limit = get_limit();
|
||||
unsigned total = 0;
|
||||
while (limit - total > 0) { // wrong: if `total` is greater than `limit` this will underflow and continue executing the loop.
|
||||
total += get_data();
|
||||
}
|
||||
@@ -0,0 +1,31 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds relational comparisons between the result of an unsigned subtraction and the value <code>0</code>.
|
||||
Such comparisons are likely wrong as the value of an unsigned subtraction can never be negative. So the
|
||||
relational comparison ends up checking whether the result of the subtraction is equal to <code>0</code>.
|
||||
This is likely not what the programmer intended.
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>If a relational comparison is intended, consider casting the result of the subtraction to a signed type.
|
||||
If the intention was to test for equality, consider replacing the relational comparison with an equality test.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<sample src="UnsignedDifferenceExpressionComparedZero.c" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>CERT C Coding Standard:
|
||||
<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
|
||||
</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,40 @@
|
||||
/**
|
||||
* @name Unsigned difference expression compared to zero
|
||||
* @description A subtraction with an unsigned result can never be negative. Using such an expression in a relational comparison with `0` is likely to be wrong.
|
||||
* @kind problem
|
||||
* @id cpp/unsigned-difference-expression-compared-zero
|
||||
* @problem.severity warning
|
||||
* @precision medium
|
||||
* @tags security
|
||||
* correctness
|
||||
* external/cwe/cwe-191
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import semmle.code.cpp.commons.Exclusions
|
||||
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
|
||||
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
|
||||
import semmle.code.cpp.controlflow.Guards
|
||||
|
||||
/** Holds if `sub` will never be negative. */
|
||||
predicate nonNegative(SubExpr sub) {
|
||||
not exprMightOverflowNegatively(sub.getFullyConverted())
|
||||
or
|
||||
// The subtraction is guarded by a check of the form `left >= right`.
|
||||
exists(GuardCondition guard, Expr left, Expr right |
|
||||
left = globalValueNumber(sub.getLeftOperand()).getAnExpr() and
|
||||
right = globalValueNumber(sub.getRightOperand()).getAnExpr() and
|
||||
guard.controls(sub.getBasicBlock(), true) and
|
||||
guard.ensuresLt(left, right, 0, sub.getBasicBlock(), false)
|
||||
)
|
||||
}
|
||||
|
||||
from RelationalOperation ro, SubExpr sub
|
||||
where
|
||||
not isFromMacroDefinition(ro) and
|
||||
not isFromMacroDefinition(sub) and
|
||||
ro.getLesserOperand().getValue().toInt() = 0 and
|
||||
ro.getGreaterOperand() = sub and
|
||||
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
|
||||
not nonNegative(sub)
|
||||
select ro, "Unsigned subtraction can never be negative."
|
||||
Reference in New Issue
Block a user