hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-02 20:25:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5076 from MathiasVP/improve-UnsignedDifferenceExpressionComparedZero

Browse Source 
C++: Improve cpp/unsigned-difference-expression-compared-zero
This commit is contained in:
Mathias Vorreiter Pedersen
2021-02-04 14:05:30 +01:00
committed by GitHub
parent a666a692f9 b55921a391
commit 9b39163411
4 changed files with 106 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql
View File

@@ -13,11 +13,28 @@
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
ro.getLesserOperand().getValue().toInt() = 0 and
ro.getGreaterOperand() = sub and
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
not nonNegative(sub)
select ro, "Difference in condition is always greater than or equal to zero"

10
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/UnsignedDifferenceExpressionComparedZero.expected Normal file
View File

@@ -0,0 +1,10 @@
| test.cpp:6:5:6:13 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:10:8:10:24 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:15:9:15:25 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:32:12:32:20 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:39:12:39:20 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:47:5:47:13 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:55:5:55:13 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:62:5:62:13 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:69:5:69:13 | ... > ... | Difference in condition is always greater than or equal to zero |
| test.cpp:75:8:75:16 | ... > ... | Difference in condition is always greater than or equal to zero |

1
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/UnsignedDifferenceExpressionComparedZero.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

77
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/test.cpp Normal file
View File

@@ -0,0 +1,77 @@
int getAnInt();
bool cond();
void test(unsigned x, unsigned y, bool unknown) {
if(x - y > 0) { } // BAD
unsigned total = getAnInt();
unsigned limit = getAnInt();
while(limit - total > 0) { // BAD
total += getAnInt();
}
if(total <= limit) {
while(limit - total > 0) { // GOOD [FALSE POSITIVE]
total += getAnInt();
if(total > limit) break;
}
}
if(x >= y) {
bool b = x - y > 0; // GOOD
}
if((int)(x - y) >= 0) { } // GOOD. Maybe an overflow happened, but the result is converted to the "likely intended" result before the comparison
if(unknown) {
y = x & 0xFF;
} else {
y = x;
}
bool b1 = x - y > 0; // GOOD [FALSE POSITIVE]
x = getAnInt();
y = getAnInt();
if(y > x) {
y = x - 1;
}
bool b2 = x - y > 0; // GOOD [FALSE POSITIVE]
int N = getAnInt();
y = x;
while(cond()) {
if(unknown) { y--; }
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
x = y;
while(cond()) {
if(unknown) break;
y--;
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
y = 0;
for(int i = 0; i < x; ++i) {
if(unknown) { ++y; }
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
x = y;
while(cond()) {
if(unknown) { x++; }
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
int n = getAnInt();
if (n > x - y) { n = x - y; }
if (n > 0) {
y += n; // NOTE: `n` is at most `x - y` at this point.
if (x - y > 0) {} // GOOD [FALSE POSITIVE]
}
}