Skip to content

Commit 9b8f558

Browse files
MathiasVPMathiasVP
authored
Merge pull request #6125 from MathiasVP/improve-tainted-arithmetic
C++: Add more barriers to `cpp/tainted-arithmetic`
2 parents 089e4e2 + 295e022 commit 9b8f558

File tree

4 files changed

+186
-102
lines changed

4 files changed

+186
-102
lines changed

‎cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name User-controlled data in arithmetic expression
33
* @description Arithmetic operations on user-controlled data that is
44
* not validated can cause overflows.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @security-severity 8.6
88
* @precision low
@@ -16,22 +16,39 @@ import cpp
1616
import semmle.code.cpp.security.Overflow
1717
import semmle.code.cpp.security.Security
1818
import semmle.code.cpp.security.TaintTracking
19+
import TaintedWithPath
20+
import Bounded
1921

20-
from Expr origin, Operation op, Expr e, string effect
22+
bindingset[op]
23+
predicate missingGuard(Operation op, Expr e, string effect) {
24+
missingGuardAgainstUnderflow(op, e) and effect = "underflow"
25+
or
26+
missingGuardAgainstOverflow(op, e) and effect = "overflow"
27+
or
28+
not e instanceof VariableAccess and effect = "overflow"
29+
}
30+
31+
class Configuration extends TaintTrackingConfiguration {
32+
override predicate isSink(Element e) {
33+
exists(Operation op |
34+
missingGuard(op, e, _) and
35+
op.getAnOperand() = e
36+
|
37+
op instanceof UnaryArithmeticOperation or
38+
op instanceof BinaryArithmeticOperation
39+
)
40+
}
41+
42+
override predicate isBarrier(Expr e) {
43+
super.isBarrier(e) or bounded(e) or e.getUnspecifiedType().(IntegralType).getSize() <= 1
44+
}
45+
}
46+
47+
from Expr origin, Expr e, string effect, PathNode sourceNode, PathNode sinkNode, Operation op
2148
where
22-
isUserInput(origin, _) and
23-
tainted(origin, e) and
49+
taintedWithPath(origin, e, sourceNode, sinkNode) and
2450
op.getAnOperand() = e and
25-
(
26-
missingGuardAgainstUnderflow(op, e) and effect = "underflow"
27-
or
28-
missingGuardAgainstOverflow(op, e) and effect = "overflow"
29-
or
30-
not e instanceof VariableAccess and effect = "overflow"
31-
) and
32-
(
33-
op instanceof UnaryArithmeticOperation or
34-
op instanceof BinaryArithmeticOperation
35-
)
36-
select e, "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
37-
origin, "User-provided value"
51+
missingGuard(op, e, effect)
52+
select e, sourceNode, sinkNode,
53+
"$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
54+
"User-provided value"

‎cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

Lines changed: 1 addition & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import cpp
1616
import semmle.code.cpp.security.Overflow
1717
import semmle.code.cpp.security.Security
1818
import semmle.code.cpp.security.TaintTracking
19-
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
2019
import TaintedWithPath
20+
import Bounded
2121

2222
predicate isUnboundedRandCall(FunctionCall fc) {
2323
exists(Function func | func = fc.getTarget() |
@@ -27,81 +27,6 @@ predicate isUnboundedRandCall(FunctionCall fc) {
2727
)
2828
}
2929

30-
/**
31-
* An operand `e` of a division expression (i.e., `e` is an operand of either a `DivExpr` or
32-
* a `AssignDivExpr`) is bounded when `e` is the left-hand side of the division.
33-
*/
34-
pragma[inline]
35-
predicate boundedDiv(Expr e, Expr left) { e = left }
36-
37-
/**
38-
* An operand `e` of a remainder expression `rem` (i.e., `rem` is either a `RemExpr` or
39-
* an `AssignRemExpr`) with left-hand side `left` and right-hand side `right` is bounded
40-
* when `e` is `left` and `right` is upper bounded by some number that is less than the maximum integer
41-
* allowed by the result type of `rem`.
42-
*/
43-
pragma[inline]
44-
predicate boundedRem(Expr e, Expr rem, Expr left, Expr right) {
45-
e = left and
46-
upperBound(right.getFullyConverted()) < exprMaxVal(rem.getFullyConverted())
47-
}
48-
49-
/**
50-
* An operand `e` of a bitwise and expression `andExpr` (i.e., `andExpr` is either an `BitwiseAndExpr`
51-
* or an `AssignAndExpr`) with operands `operand1` and `operand2` is the operand that is not `e` is upper
52-
* bounded by some number that is less than the maximum integer allowed by the result type of `andExpr`.
53-
*/
54-
pragma[inline]
55-
predicate boundedBitwiseAnd(Expr e, Expr andExpr, Expr operand1, Expr operand2) {
56-
operand1 != operand2 and
57-
e = operand1 and
58-
upperBound(operand2.getFullyConverted()) < exprMaxVal(andExpr.getFullyConverted())
59-
}
60-
61-
/**
62-
* Holds if `e` is an arithmetic expression that cannot overflow, or if `e` is an operand of an
63-
* operation that may greatly reduces the range of possible values.
64-
*/
65-
predicate bounded(Expr e) {
66-
(
67-
e instanceof UnaryArithmeticOperation or
68-
e instanceof BinaryArithmeticOperation or
69-
e instanceof AssignArithmeticOperation
70-
) and
71-
not convertedExprMightOverflow(e)
72-
or
73-
// For `%` and `&` we require that `e` is bounded by a value that is strictly smaller than the
74-
// maximum possible value of the result type of the operation.
75-
// For example, the function call `rand()` is considered bounded in the following program:
76-
// ```
77-
// int i = rand() % (UINT8_MAX + 1);
78-
// ```
79-
// but not in:
80-
// ```
81-
// unsigned char uc = rand() % (UINT8_MAX + 1);
82-
// ```
83-
exists(RemExpr rem | boundedRem(e, rem, rem.getLeftOperand(), rem.getRightOperand()))
84-
or
85-
exists(AssignRemExpr rem | boundedRem(e, rem, rem.getLValue(), rem.getRValue()))
86-
or
87-
exists(BitwiseAndExpr andExpr |
88-
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
89-
)
90-
or
91-
exists(AssignAndExpr andExpr |
92-
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
93-
)
94-
or
95-
// Optimitically assume that a division or right shift always yields a much smaller value.
96-
boundedDiv(e, any(DivExpr div).getLeftOperand())
97-
or
98-
boundedDiv(e, any(AssignDivExpr div).getLValue())
99-
or
100-
boundedDiv(e, any(RShiftExpr shift).getLeftOperand())
101-
or
102-
boundedDiv(e, any(AssignRShiftExpr div).getLValue())
103-
}
104-
10530
predicate isUnboundedRandCallOrParent(Expr e) {
10631
isUnboundedRandCall(e)
10732
or

‎cpp/ql/src/Security/CWE/CWE-190/Bounded.qll

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* This file provides the `bounded` predicate that is used in both `cpp/uncontrolled-arithmetic`
3+
* and `cpp/tainted-arithmetic`.
4+
*/
5+
6+
private import cpp
7+
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
8+
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
9+
10+
/**
11+
* An operand `e` of a division expression (i.e., `e` is an operand of either a `DivExpr` or
12+
* a `AssignDivExpr`) is bounded when `e` is the left-hand side of the division.
13+
*/
14+
pragma[inline]
15+
private predicate boundedDiv(Expr e, Expr left) { e = left }
16+
17+
/**
18+
* An operand `e` of a remainder expression `rem` (i.e., `rem` is either a `RemExpr` or
19+
* an `AssignRemExpr`) with left-hand side `left` and right-ahnd side `right` is bounded
20+
* when `e` is `left` and `right` is upper bounded by some number that is less than the maximum integer
21+
* allowed by the result type of `rem`.
22+
*/
23+
pragma[inline]
24+
private predicate boundedRem(Expr e, Expr rem, Expr left, Expr right) {
25+
e = left and
26+
upperBound(right.getFullyConverted()) < exprMaxVal(rem.getFullyConverted())
27+
}
28+
29+
/**
30+
* An operand `e` of a bitwise and expression `andExpr` (i.e., `andExpr` is either an `BitwiseAndExpr`
31+
* or an `AssignAndExpr`) with operands `operand1` and `operand2` is the operand that is not `e` is upper
32+
* bounded by some number that is less than the maximum integer allowed by the result type of `andExpr`.
33+
*/
34+
pragma[inline]
35+
private predicate boundedBitwiseAnd(Expr e, Expr andExpr, Expr operand1, Expr operand2) {
36+
operand1 != operand2 and
37+
e = operand1 and
38+
upperBound(operand2.getFullyConverted()) < exprMaxVal(andExpr.getFullyConverted())
39+
}
40+
41+
/**
42+
* Holds if `e` is an arithmetic expression that cannot overflow, or if `e` is an operand of an
43+
* operation that may greatly reduce the range of possible values.
44+
*/
45+
predicate bounded(Expr e) {
46+
(
47+
e instanceof UnaryArithmeticOperation or
48+
e instanceof BinaryArithmeticOperation or
49+
e instanceof AssignArithmeticOperation
50+
) and
51+
not convertedExprMightOverflow(e)
52+
or
53+
// For `%` and `&` we require that `e` is bounded by a value that is strictly smaller than the
54+
// maximum possible value of the result type of the operation.
55+
// For example, the function call `rand()` is considered bounded in the following program:
56+
// ```
57+
// int i = rand() % (UINT8_MAX + 1);
58+
// ```
59+
// but not in:
60+
// ```
61+
// unsigned char uc = rand() % (UINT8_MAX + 1);
62+
// ```
63+
exists(RemExpr rem | boundedRem(e, rem, rem.getLeftOperand(), rem.getRightOperand()))
64+
or
65+
exists(AssignRemExpr rem | boundedRem(e, rem, rem.getLValue(), rem.getRValue()))
66+
or
67+
exists(BitwiseAndExpr andExpr |
68+
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
69+
)
70+
or
71+
exists(AssignAndExpr andExpr |
72+
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
73+
)
74+
or
75+
// Optimitically assume that a division always yields a much smaller value.
76+
boundedDiv(e, any(DivExpr div).getLeftOperand())
77+
or
78+
boundedDiv(e, any(AssignDivExpr div).getLValue())
79+
or
80+
boundedDiv(e, any(RShiftExpr shift).getLeftOperand())
81+
or
82+
boundedDiv(e, any(AssignRShiftExpr div).getLValue())
83+
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,68 @@
1-
| test2.cpp:14:11:14:11 | v | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
2-
| test2.cpp:14:11:14:11 | v | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
3-
| test5.cpp:17:6:17:18 | call to getTaintedInt | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
4-
| test5.cpp:19:6:19:6 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
5-
| test5.cpp:19:6:19:6 | y | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
6-
| test.c:14:15:14:28 | maxConnections | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:11:29:11:32 | argv | User-provided value |
7-
| test.c:14:15:14:28 | maxConnections | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:11:29:11:32 | argv | User-provided value |
8-
| test.c:44:7:44:10 | len2 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:41:17:41:20 | argv | User-provided value |
9-
| test.c:54:7:54:10 | len3 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:51:17:51:20 | argv | User-provided value |
1+
edges
2+
| test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v |
3+
| test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v |
4+
| test2.cpp:25:22:25:23 | & ... | test2.cpp:27:2:27:11 | v |
5+
| test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:27:2:27:11 | v |
6+
| test2.cpp:27:2:27:11 | v | test2.cpp:12:21:12:21 | v |
7+
| test5.cpp:9:7:9:9 | buf | test5.cpp:10:9:10:27 | Store |
8+
| test5.cpp:9:7:9:9 | gets output argument | test5.cpp:10:9:10:27 | Store |
9+
| test5.cpp:10:9:10:27 | Store | test5.cpp:17:6:17:18 | call to getTaintedInt |
10+
| test5.cpp:10:9:10:27 | Store | test5.cpp:17:6:17:18 | call to getTaintedInt |
11+
| test5.cpp:10:9:10:27 | Store | test5.cpp:18:6:18:18 | call to getTaintedInt |
12+
| test5.cpp:18:6:18:18 | call to getTaintedInt | test5.cpp:19:6:19:6 | y |
13+
| test5.cpp:18:6:18:18 | call to getTaintedInt | test5.cpp:19:6:19:6 | y |
14+
| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections |
15+
| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections |
16+
| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections |
17+
| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections |
18+
| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 |
19+
| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 |
20+
| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 |
21+
| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 |
22+
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
23+
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
24+
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
25+
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
26+
nodes
27+
| test2.cpp:12:21:12:21 | v | semmle.label | v |
28+
| test2.cpp:14:11:14:11 | v | semmle.label | v |
29+
| test2.cpp:14:11:14:11 | v | semmle.label | v |
30+
| test2.cpp:14:11:14:11 | v | semmle.label | v |
31+
| test2.cpp:25:22:25:23 | & ... | semmle.label | & ... |
32+
| test2.cpp:25:22:25:23 | fscanf output argument | semmle.label | fscanf output argument |
33+
| test2.cpp:27:2:27:11 | v | semmle.label | v |
34+
| test5.cpp:9:7:9:9 | buf | semmle.label | buf |
35+
| test5.cpp:9:7:9:9 | gets output argument | semmle.label | gets output argument |
36+
| test5.cpp:10:9:10:27 | Store | semmle.label | Store |
37+
| test5.cpp:17:6:17:18 | call to getTaintedInt | semmle.label | call to getTaintedInt |
38+
| test5.cpp:17:6:17:18 | call to getTaintedInt | semmle.label | call to getTaintedInt |
39+
| test5.cpp:17:6:17:18 | call to getTaintedInt | semmle.label | call to getTaintedInt |
40+
| test5.cpp:18:6:18:18 | call to getTaintedInt | semmle.label | call to getTaintedInt |
41+
| test5.cpp:19:6:19:6 | y | semmle.label | y |
42+
| test5.cpp:19:6:19:6 | y | semmle.label | y |
43+
| test5.cpp:19:6:19:6 | y | semmle.label | y |
44+
| test.c:11:29:11:32 | argv | semmle.label | argv |
45+
| test.c:11:29:11:32 | argv | semmle.label | argv |
46+
| test.c:14:15:14:28 | maxConnections | semmle.label | maxConnections |
47+
| test.c:14:15:14:28 | maxConnections | semmle.label | maxConnections |
48+
| test.c:14:15:14:28 | maxConnections | semmle.label | maxConnections |
49+
| test.c:41:17:41:20 | argv | semmle.label | argv |
50+
| test.c:41:17:41:20 | argv | semmle.label | argv |
51+
| test.c:44:7:44:10 | len2 | semmle.label | len2 |
52+
| test.c:44:7:44:10 | len2 | semmle.label | len2 |
53+
| test.c:44:7:44:10 | len2 | semmle.label | len2 |
54+
| test.c:51:17:51:20 | argv | semmle.label | argv |
55+
| test.c:51:17:51:20 | argv | semmle.label | argv |
56+
| test.c:54:7:54:10 | len3 | semmle.label | len3 |
57+
| test.c:54:7:54:10 | len3 | semmle.label | len3 |
58+
| test.c:54:7:54:10 | len3 | semmle.label | len3 |
59+
#select
60+
| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
61+
| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
62+
| test5.cpp:17:6:17:18 | call to getTaintedInt | test5.cpp:9:7:9:9 | buf | test5.cpp:17:6:17:18 | call to getTaintedInt | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
63+
| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
64+
| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
65+
| test.c:14:15:14:28 | maxConnections | test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:11:29:11:32 | argv | User-provided value |
66+
| test.c:14:15:14:28 | maxConnections | test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:11:29:11:32 | argv | User-provided value |
67+
| test.c:44:7:44:10 | len2 | test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:41:17:41:20 | argv | User-provided value |
68+
| test.c:54:7:54:10 | len3 | test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:51:17:51:20 | argv | User-provided value |

0 commit comments

Comments
 (0)