mirror of
https://github.com/github/codeql.git
synced 2026-04-28 18:25:24 +02:00
Merge pull request #5887 from MathiasVP/fewer-rand-sources-in-uncontrolled-arithmetic
C++: Add more sanitizers to `cpp/uncontrolled-arithmetic`
This commit is contained in:
Robert Marsh
2
cpp/change-notes/2021-12-05-uncontrolled-arithmetic.md
Normal file
2
cpp/change-notes/2021-12-05-uncontrolled-arithmetic.md
Normal file
@@ -0,0 +1,2 @@
|
||||
lgtm,codescanning
|
||||
* The query "Uncontrolled arithmetic" (`cpp/uncontrolled-arithmetic`) has been improved to produce fewer false positives.
|
||||
@@ -15,34 +15,99 @@ import cpp
|
||||
import semmle.code.cpp.security.Overflow
|
||||
import semmle.code.cpp.security.Security
|
||||
import semmle.code.cpp.security.TaintTracking
|
||||
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
|
||||
import TaintedWithPath
|
||||
|
||||
predicate isRandCall(FunctionCall fc) { fc.getTarget().getName() = "rand" }
|
||||
|
||||
predicate isRandCallOrParent(Expr e) {
|
||||
isRandCall(e) or
|
||||
isRandCallOrParent(e.getAChild())
|
||||
predicate isUnboundedRandCall(FunctionCall fc) {
|
||||
fc.getTarget().getName() = "rand" and not bounded(fc)
|
||||
}
|
||||
|
||||
predicate isRandValue(Expr e) {
|
||||
isRandCall(e)
|
||||
/**
|
||||
* 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]
|
||||
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]
|
||||
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
|
||||
* bounded by some number that is less than the maximum integer allowed by the result type of `andExpr`.
|
||||
*/
|
||||
pragma[inline]
|
||||
predicate boundedBitwiseAnd(Expr e, Expr andExpr, Expr operand1, Expr operand2) {
|
||||
operand1 != operand2 and
|
||||
e = operand1 and
|
||||
upperBound(operand2.getFullyConverted()) < exprMaxVal(andExpr.getFullyConverted())
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fc` is a part of the left operand of a binary operation that greatly reduces the range
|
||||
* of possible values.
|
||||
*/
|
||||
predicate bounded(Expr e) {
|
||||
// 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()))
|
||||
or
|
||||
exists(AssignRemExpr rem | boundedRem(e, rem, rem.getLValue(), rem.getRValue()))
|
||||
or
|
||||
exists(BitwiseAndExpr andExpr |
|
||||
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
|
||||
)
|
||||
or
|
||||
exists(AssignAndExpr andExpr |
|
||||
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
|
||||
)
|
||||
or
|
||||
// Optimitically assume that a division always yields a much smaller value.
|
||||
boundedDiv(e, any(DivExpr div).getLeftOperand())
|
||||
or
|
||||
boundedDiv(e, any(AssignDivExpr div).getLValue())
|
||||
}
|
||||
|
||||
predicate isUnboundedRandCallOrParent(Expr e) {
|
||||
isUnboundedRandCall(e)
|
||||
or
|
||||
isUnboundedRandCallOrParent(e.getAChild())
|
||||
}
|
||||
|
||||
predicate isUnboundedRandValue(Expr e) {
|
||||
isUnboundedRandCall(e)
|
||||
or
|
||||
exists(MacroInvocation mi |
|
||||
e = mi.getExpr() and
|
||||
isRandCallOrParent(e)
|
||||
isUnboundedRandCallOrParent(e)
|
||||
)
|
||||
}
|
||||
|
||||
class SecurityOptionsArith extends SecurityOptions {
|
||||
override predicate isUserInput(Expr expr, string cause) {
|
||||
isRandValue(expr) and
|
||||
cause = "rand" and
|
||||
not expr.getParent*() instanceof DivExpr
|
||||
isUnboundedRandValue(expr) and
|
||||
cause = "rand"
|
||||
}
|
||||
}
|
||||
|
||||
predicate isDiv(VariableAccess va) { exists(AssignDivExpr div | div.getLValue() = va) }
|
||||
|
||||
predicate missingGuard(VariableAccess va, string effect) {
|
||||
exists(Operation op | op.getAnOperand() = va |
|
||||
missingGuardAgainstUnderflow(op, va) and effect = "underflow"
|
||||
@@ -52,29 +117,15 @@ predicate missingGuard(VariableAccess va, string effect) {
|
||||
}
|
||||
|
||||
class Configuration extends TaintTrackingConfiguration {
|
||||
override predicate isSink(Element e) {
|
||||
isDiv(e)
|
||||
or
|
||||
missingGuard(e, _)
|
||||
}
|
||||
}
|
||||
override predicate isSink(Element e) { missingGuard(e, _) }
|
||||
|
||||
/**
|
||||
* A value that undergoes division is likely to be bounded within a safe
|
||||
* range.
|
||||
*/
|
||||
predicate guardedByAssignDiv(Expr origin) {
|
||||
exists(VariableAccess va |
|
||||
taintedWithPath(origin, va, _, _) and
|
||||
isDiv(va)
|
||||
)
|
||||
override predicate isBarrier(Expr e) { super.isBarrier(e) or bounded(e) }
|
||||
}
|
||||
|
||||
from Expr origin, VariableAccess va, string effect, PathNode sourceNode, PathNode sinkNode
|
||||
where
|
||||
taintedWithPath(origin, va, sourceNode, sinkNode) and
|
||||
missingGuard(va, effect) and
|
||||
not guardedByAssignDiv(origin)
|
||||
missingGuard(va, effect)
|
||||
select va, sourceNode, sinkNode,
|
||||
"$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
|
||||
"Uncontrolled value"
|
||||
|
||||
@@ -7,30 +7,10 @@ edges
|
||||
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
|
||||
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
|
||||
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
|
||||
| test.c:39:13:39:21 | ... % ... | test.c:40:5:40:5 | r |
|
||||
| test.c:39:13:39:21 | ... % ... | test.c:40:5:40:5 | r |
|
||||
| test.c:39:13:39:21 | ... % ... | test.c:40:5:40:5 | r |
|
||||
| test.c:39:13:39:21 | ... % ... | test.c:40:5:40:5 | r |
|
||||
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
|
||||
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
|
||||
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
|
||||
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
|
||||
| test.c:54:13:54:16 | call to rand | test.c:56:5:56:5 | r |
|
||||
| test.c:54:13:54:16 | call to rand | test.c:56:5:56:5 | r |
|
||||
| test.c:54:13:54:16 | call to rand | test.c:56:5:56:5 | r |
|
||||
| test.c:54:13:54:16 | call to rand | test.c:56:5:56:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:61:5:61:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:61:5:61:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:61:5:61:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:61:5:61:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:62:5:62:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:62:5:62:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:62:5:62:5 | r |
|
||||
| test.c:60:13:60:16 | call to rand | test.c:62:5:62:5 | r |
|
||||
| test.c:66:13:66:16 | call to rand | test.c:67:5:67:5 | r |
|
||||
| test.c:66:13:66:16 | call to rand | test.c:67:5:67:5 | r |
|
||||
| test.c:66:13:66:16 | call to rand | test.c:67:5:67:5 | r |
|
||||
| test.c:66:13:66:16 | call to rand | test.c:67:5:67:5 | r |
|
||||
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
|
||||
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
|
||||
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
|
||||
@@ -67,34 +47,11 @@ nodes
|
||||
| test.c:35:5:35:5 | r | semmle.label | r |
|
||||
| test.c:35:5:35:5 | r | semmle.label | r |
|
||||
| test.c:35:5:35:5 | r | semmle.label | r |
|
||||
| test.c:39:13:39:21 | ... % ... | semmle.label | ... % ... |
|
||||
| test.c:39:13:39:21 | ... % ... | semmle.label | ... % ... |
|
||||
| test.c:40:5:40:5 | r | semmle.label | r |
|
||||
| test.c:40:5:40:5 | r | semmle.label | r |
|
||||
| test.c:40:5:40:5 | r | semmle.label | r |
|
||||
| test.c:44:13:44:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:44:13:44:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:45:5:45:5 | r | semmle.label | r |
|
||||
| test.c:45:5:45:5 | r | semmle.label | r |
|
||||
| test.c:45:5:45:5 | r | semmle.label | r |
|
||||
| test.c:54:13:54:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:54:13:54:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:56:5:56:5 | r | semmle.label | r |
|
||||
| test.c:56:5:56:5 | r | semmle.label | r |
|
||||
| test.c:56:5:56:5 | r | semmle.label | r |
|
||||
| test.c:60:13:60:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:60:13:60:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:61:5:61:5 | r | semmle.label | r |
|
||||
| test.c:61:5:61:5 | r | semmle.label | r |
|
||||
| test.c:61:5:61:5 | r | semmle.label | r |
|
||||
| test.c:62:5:62:5 | r | semmle.label | r |
|
||||
| test.c:62:5:62:5 | r | semmle.label | r |
|
||||
| test.c:62:5:62:5 | r | semmle.label | r |
|
||||
| test.c:66:13:66:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:66:13:66:16 | call to rand | semmle.label | call to rand |
|
||||
| test.c:67:5:67:5 | r | semmle.label | r |
|
||||
| test.c:67:5:67:5 | r | semmle.label | r |
|
||||
| test.c:67:5:67:5 | r | semmle.label | r |
|
||||
| test.c:75:13:75:19 | ... ^ ... | semmle.label | ... ^ ... |
|
||||
| test.c:75:13:75:19 | ... ^ ... | semmle.label | ... ^ ... |
|
||||
| test.c:77:9:77:9 | r | semmle.label | r |
|
||||
@@ -133,10 +90,7 @@ nodes
|
||||
#select
|
||||
| test.c:21:17:21:17 | r | test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:18:13:18:16 | call to rand | Uncontrolled value |
|
||||
| test.c:35:5:35:5 | r | test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:34:13:34:18 | call to rand | Uncontrolled value |
|
||||
| test.c:40:5:40:5 | r | test.c:39:13:39:21 | ... % ... | test.c:40:5:40:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:39:13:39:21 | ... % ... | Uncontrolled value |
|
||||
| test.c:45:5:45:5 | r | test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:44:13:44:16 | call to rand | Uncontrolled value |
|
||||
| test.c:56:5:56:5 | r | test.c:54:13:54:16 | call to rand | test.c:56:5:56:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:54:13:54:16 | call to rand | Uncontrolled value |
|
||||
| test.c:67:5:67:5 | r | test.c:66:13:66:16 | call to rand | test.c:67:5:67:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:66:13:66:16 | call to rand | Uncontrolled value |
|
||||
| test.c:77:9:77:9 | r | test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:75:13:75:19 | ... ^ ... | Uncontrolled value |
|
||||
| test.c:100:5:100:5 | r | test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:99:14:99:19 | call to rand | Uncontrolled value |
|
||||
| test.cpp:25:7:25:7 | r | test.cpp:8:9:8:12 | call to rand | test.cpp:25:7:25:7 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:8:9:8:12 | call to rand | Uncontrolled value |
|
||||
|
||||
@@ -37,7 +37,7 @@ void randomTester() {
|
||||
|
||||
{
|
||||
int r = RANDN(100);
|
||||
r += 100; // GOOD: The return from RANDN is bounded [FALSE POSITIVE]
|
||||
r += 100; // GOOD: The return from RANDN is bounded
|
||||
}
|
||||
|
||||
{
|
||||
@@ -53,7 +53,7 @@ void randomTester() {
|
||||
{
|
||||
int r = rand();
|
||||
r = r / 10;
|
||||
r += 100; // GOOD [FALSE POSITIVE]
|
||||
r += 100; // GOOD
|
||||
}
|
||||
|
||||
{
|
||||
@@ -64,7 +64,7 @@ void randomTester() {
|
||||
|
||||
{
|
||||
int r = rand() & 0xFF;
|
||||
r += 100; // GOOD [FALSE POSITIVE]
|
||||
r += 100; // GOOD
|
||||
}
|
||||
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user