Skip to content

C++: More effective barriers in the bounded predicate for CWE-190 #6159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 12, 2021
Merged

C++: More effective barriers in the bounded predicate for CWE-190 #6159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6f1f8d
C++: Add testcases demonstrating FPs from real code.
MathiasVP Jun 25, 2021
4fc60ae
C++: Relax the restrictions on when '%' is a barrier and accept test …
MathiasVP Jun 25, 2021
7da7ec6
C++: Inline predicates from 'Bounded.qll'.
MathiasVP Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 7 additions & 35 deletions cpp/ql/src/Security/CWE/CWE-190/Bounded.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,6 @@ private import cpp
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils

/**
* An operand `e` of a division expression (i.e., `e` is an operand of either a `DivExpr` or
* a `AssignDivExpr`) is bounded when `e` is the left-hand side of the division.
*/
pragma[inline]
private predicate boundedDiv(Expr e, Expr left) { e = left }

/**
* An operand `e` of a remainder expression `rem` (i.e., `rem` is either a `RemExpr` or
* an `AssignRemExpr`) with left-hand side `left` and right-ahnd side `right` is bounded
* when `e` is `left` and `right` is upper bounded by some number that is less than the maximum integer
* allowed by the result type of `rem`.
*/
pragma[inline]
private predicate boundedRem(Expr e, Expr rem, Expr left, Expr right) {
e = left and
upperBound(right.getFullyConverted()) < exprMaxVal(rem.getFullyConverted())
}

/**
* An operand `e` of a bitwise and expression `andExpr` (i.e., `andExpr` is either an `BitwiseAndExpr`
* or an `AssignAndExpr`) with operands `operand1` and `operand2` is the operand that is not `e` is upper
Expand All @@ -50,19 +31,10 @@ predicate bounded(Expr e) {
) and
not convertedExprMightOverflow(e)
or
// For `%` and `&` we require that `e` is bounded by a value that is strictly smaller than the
// maximum possible value of the result type of the operation.
// For example, the function call `rand()` is considered bounded in the following program:
// ```
// int i = rand() % (UINT8_MAX + 1);
// ```
// but not in:
// ```
// unsigned char uc = rand() % (UINT8_MAX + 1);
// ```
exists(RemExpr rem | boundedRem(e, rem, rem.getLeftOperand(), rem.getRightOperand()))
// Optimitically assume that a remainder expression always yields a much smaller value.
e = any(RemExpr rem).getLeftOperand()
or
exists(AssignRemExpr rem | boundedRem(e, rem, rem.getLValue(), rem.getRValue()))
e = any(AssignRemExpr rem).getLValue()
or
exists(BitwiseAndExpr andExpr |
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
Expand All @@ -73,11 +45,11 @@ predicate bounded(Expr e) {
)
or
// Optimitically assume that a division always yields a much smaller value.
boundedDiv(e, any(DivExpr div).getLeftOperand())
e = any(DivExpr div).getLeftOperand()
or
boundedDiv(e, any(AssignDivExpr div).getLValue())
e = any(AssignDivExpr div).getLValue()
or
boundedDiv(e, any(RShiftExpr shift).getLeftOperand())
e = any(RShiftExpr shift).getLeftOperand()
or
boundedDiv(e, any(AssignRShiftExpr div).getLValue())
e = any(AssignRShiftExpr div).getLValue()
}
11 changes: 10 additions & 1 deletion cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,13 @@ void randomTester() {

void add_100(int r) {
r += 100; // GOOD
}
}

void randomTester2(int bound, int min, int max) {
int r1 = rand() % bound;
r1 += 100; // GOOD (`bound` may possibly be MAX_INT in which case this could
// still overflow, but it's most likely fine)

int r2 = (rand() % (max - min + 1)) + min;
r2 += 100; // GOOD (This is a common way to clamp the random value between [min, max])
}