Skip to content

Commit 6a11aa7

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#6154 from MathiasVP/more-random-sources-in-uncontrolled-arithmetic
C++: Add more random sources in `cpp/uncontrolled-arithmetic`
2 parents c47d680 + dec747f commit 6a11aa7

File tree

4 files changed

+101
-82
lines changed

4 files changed

+101
-82
lines changed

‎cpp/change-notes/2021-06-24-uncontrolled-arithmetic.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm
2+
* The 'Uncontrolled data in arithmetic expression' (cpp/uncontrolled-arithmetic) query now recognizes more sources of randomness.

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

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,38 +15,61 @@
1515
import cpp
1616
import semmle.code.cpp.security.Overflow
1717
import semmle.code.cpp.security.Security
18-
import semmle.code.cpp.security.TaintTracking
19-
import TaintedWithPath
18+
import semmle.code.cpp.security.FlowSources
19+
import semmle.code.cpp.ir.dataflow.TaintTracking
20+
import DataFlow::PathGraph
2021
import Bounded
2122

22-
predicate isUnboundedRandCall(FunctionCall fc) {
23-
exists(Function func | func = fc.getTarget() |
24-
func.hasGlobalOrStdOrBslName("rand") and
25-
not bounded(fc) and
26-
func.getNumberOfParameters() = 0
27-
)
23+
/**
24+
* A function that outputs random data such as `std::rand`.
25+
*/
26+
abstract class RandomFunction extends Function {
27+
/**
28+
* Gets the `FunctionOutput` that describes how this function returns the random data.
29+
*/
30+
FunctionOutput getFunctionOutput() { result.isReturnValue() }
31+
}
32+
33+
/**
34+
* The standard function `std::rand`.
35+
*/
36+
private class StdRand extends RandomFunction {
37+
StdRand() {
38+
this.hasGlobalOrStdOrBslName("rand") and
39+
this.getNumberOfParameters() = 0
40+
}
2841
}
2942

30-
predicate isUnboundedRandCallOrParent(Expr e) {
31-
isUnboundedRandCall(e)
32-
or
33-
isUnboundedRandCallOrParent(e.getAChild())
43+
/**
44+
* The Unix function `rand_r`.
45+
*/
46+
private class RandR extends RandomFunction {
47+
RandR() {
48+
this.hasGlobalName("rand_r") and
49+
this.getNumberOfParameters() = 1
50+
}
3451
}
3552

36-
predicate isUnboundedRandValue(Expr e) {
37-
isUnboundedRandCall(e)
38-
or
39-
exists(MacroInvocation mi |
40-
e = mi.getExpr() and
41-
isUnboundedRandCallOrParent(e)
42-
)
53+
/**
54+
* The Unix function `random`.
55+
*/
56+
private class Random extends RandomFunction {
57+
Random() {
58+
this.hasGlobalName("random") and
59+
this.getNumberOfParameters() = 1
60+
}
4361
}
4462

45-
class SecurityOptionsArith extends SecurityOptions {
46-
override predicate isUserInput(Expr expr, string cause) {
47-
isUnboundedRandValue(expr) and
48-
cause = "rand"
63+
/**
64+
* The Windows `rand_s` function.
65+
*/
66+
private class RandS extends RandomFunction {
67+
RandS() {
68+
this.hasGlobalName("rand_s") and
69+
this.getNumberOfParameters() = 1
4970
}
71+
72+
override FunctionOutput getFunctionOutput() { result.isParameterDeref(0) }
5073
}
5174

5275
predicate missingGuard(VariableAccess va, string effect) {
@@ -57,16 +80,47 @@ predicate missingGuard(VariableAccess va, string effect) {
5780
)
5881
}
5982

60-
class Configuration extends TaintTrackingConfiguration {
61-
override predicate isSink(Element e) { missingGuard(e, _) }
83+
class UncontrolledArithConfiguration extends TaintTracking::Configuration {
84+
UncontrolledArithConfiguration() { this = "UncontrolledArithConfiguration" }
6285

63-
override predicate isBarrier(Expr e) { super.isBarrier(e) or bounded(e) }
86+
override predicate isSource(DataFlow::Node source) {
87+
exists(RandomFunction rand, Call call | call.getTarget() = rand |
88+
rand.getFunctionOutput().isReturnValue() and
89+
source.asExpr() = call
90+
or
91+
exists(int n |
92+
source.asDefiningArgument() = call.getArgument(n) and
93+
rand.getFunctionOutput().isParameterDeref(n)
94+
)
95+
)
96+
}
97+
98+
override predicate isSink(DataFlow::Node sink) { missingGuard(sink.asExpr(), _) }
99+
100+
override predicate isSanitizer(DataFlow::Node node) {
101+
bounded(node.asExpr())
102+
or
103+
// If this expression is part of bitwise 'and' or 'or' operation it's likely that the value is
104+
// only used as a bit pattern.
105+
node.asExpr() =
106+
any(Operation op |
107+
op instanceof BitwiseOrExpr or
108+
op instanceof BitwiseAndExpr or
109+
op instanceof ComplementExpr
110+
).getAnOperand*()
111+
}
64112
}
65113

66-
from Expr origin, VariableAccess va, string effect, PathNode sourceNode, PathNode sinkNode
114+
/** Gets the expression that corresponds to `node`, if any. */
115+
Expr getExpr(DataFlow::Node node) { result = [node.asExpr(), node.asDefiningArgument()] }
116+
117+
from
118+
UncontrolledArithConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
119+
VariableAccess va, string effect
67120
where
68-
taintedWithPath(origin, va, sourceNode, sinkNode) and
121+
config.hasFlowPath(source, sink) and
122+
sink.getNode().asExpr() = va and
69123
missingGuard(va, effect)
70-
select va, sourceNode, sinkNode,
71-
"$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
72-
"Uncontrolled value"
124+
select sink.getNode(), source, sink,
125+
"$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
126+
getExpr(source.getNode()), "Uncontrolled value"

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/ArithmeticUncontrolled.expected

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,60 @@
11
edges
22
| test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r |
3-
| test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r |
4-
| test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r |
5-
| test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r |
6-
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
7-
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
8-
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
93
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
104
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
11-
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
12-
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
13-
| test.c:44:13:44:16 | call to rand | test.c:45:5:45:5 | r |
14-
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
15-
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
16-
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
17-
| test.c:75:13:75:19 | ... ^ ... | test.c:77:9:77:9 | r |
18-
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
19-
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
20-
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
5+
| test.c:75:13:75:19 | call to rand | test.c:77:9:77:9 | r |
6+
| test.c:75:13:75:19 | call to rand | test.c:77:9:77:9 | r |
7+
| test.c:81:14:81:17 | call to rand | test.c:83:9:83:9 | r |
8+
| test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r |
219
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
2210
| test.cpp:8:9:8:12 | Store | test.cpp:24:11:24:18 | call to get_rand |
2311
| test.cpp:8:9:8:12 | call to rand | test.cpp:8:9:8:12 | Store |
24-
| test.cpp:8:9:8:12 | call to rand | test.cpp:8:9:8:12 | Store |
2512
| test.cpp:13:2:13:15 | Chi [[]] | test.cpp:30:13:30:14 | get_rand2 output argument [[]] |
2613
| test.cpp:13:10:13:13 | call to rand | test.cpp:13:2:13:15 | Chi [[]] |
27-
| test.cpp:13:10:13:13 | call to rand | test.cpp:13:2:13:15 | Chi [[]] |
2814
| test.cpp:18:2:18:14 | Chi [[]] | test.cpp:36:13:36:13 | get_rand3 output argument [[]] |
2915
| test.cpp:18:9:18:12 | call to rand | test.cpp:18:2:18:14 | Chi [[]] |
30-
| test.cpp:18:9:18:12 | call to rand | test.cpp:18:2:18:14 | Chi [[]] |
31-
| test.cpp:24:11:24:18 | call to get_rand | test.cpp:25:7:25:7 | r |
3216
| test.cpp:24:11:24:18 | call to get_rand | test.cpp:25:7:25:7 | r |
3317
| test.cpp:30:13:30:14 | Chi | test.cpp:31:7:31:7 | r |
34-
| test.cpp:30:13:30:14 | Chi | test.cpp:31:7:31:7 | r |
3518
| test.cpp:30:13:30:14 | get_rand2 output argument [[]] | test.cpp:30:13:30:14 | Chi |
3619
| test.cpp:36:13:36:13 | Chi | test.cpp:37:7:37:7 | r |
37-
| test.cpp:36:13:36:13 | Chi | test.cpp:37:7:37:7 | r |
3820
| test.cpp:36:13:36:13 | get_rand3 output argument [[]] | test.cpp:36:13:36:13 | Chi |
3921
nodes
4022
| test.c:18:13:18:16 | call to rand | semmle.label | call to rand |
41-
| test.c:18:13:18:16 | call to rand | semmle.label | call to rand |
42-
| test.c:21:17:21:17 | r | semmle.label | r |
43-
| test.c:21:17:21:17 | r | semmle.label | r |
4423
| test.c:21:17:21:17 | r | semmle.label | r |
4524
| test.c:34:13:34:18 | call to rand | semmle.label | call to rand |
46-
| test.c:34:13:34:18 | call to rand | semmle.label | call to rand |
47-
| test.c:35:5:35:5 | r | semmle.label | r |
48-
| test.c:35:5:35:5 | r | semmle.label | r |
4925
| test.c:35:5:35:5 | r | semmle.label | r |
5026
| test.c:44:13:44:16 | call to rand | semmle.label | call to rand |
51-
| test.c:44:13:44:16 | call to rand | semmle.label | call to rand |
52-
| test.c:45:5:45:5 | r | semmle.label | r |
5327
| test.c:45:5:45:5 | r | semmle.label | r |
54-
| test.c:45:5:45:5 | r | semmle.label | r |
55-
| test.c:75:13:75:19 | ... ^ ... | semmle.label | ... ^ ... |
56-
| test.c:75:13:75:19 | ... ^ ... | semmle.label | ... ^ ... |
57-
| test.c:77:9:77:9 | r | semmle.label | r |
58-
| test.c:77:9:77:9 | r | semmle.label | r |
28+
| test.c:75:13:75:19 | call to rand | semmle.label | call to rand |
29+
| test.c:75:13:75:19 | call to rand | semmle.label | call to rand |
5930
| test.c:77:9:77:9 | r | semmle.label | r |
31+
| test.c:81:14:81:17 | call to rand | semmle.label | call to rand |
32+
| test.c:81:23:81:26 | call to rand | semmle.label | call to rand |
33+
| test.c:83:9:83:9 | r | semmle.label | r |
6034
| test.c:99:14:99:19 | call to rand | semmle.label | call to rand |
61-
| test.c:99:14:99:19 | call to rand | semmle.label | call to rand |
62-
| test.c:100:5:100:5 | r | semmle.label | r |
63-
| test.c:100:5:100:5 | r | semmle.label | r |
6435
| test.c:100:5:100:5 | r | semmle.label | r |
6536
| test.cpp:8:9:8:12 | Store | semmle.label | Store |
6637
| test.cpp:8:9:8:12 | call to rand | semmle.label | call to rand |
67-
| test.cpp:8:9:8:12 | call to rand | semmle.label | call to rand |
6838
| test.cpp:13:2:13:15 | Chi [[]] | semmle.label | Chi [[]] |
69-
| test.cpp:13:2:13:15 | ChiPartial | semmle.label | ChiPartial |
70-
| test.cpp:13:10:13:13 | call to rand | semmle.label | call to rand |
7139
| test.cpp:13:10:13:13 | call to rand | semmle.label | call to rand |
7240
| test.cpp:18:2:18:14 | Chi [[]] | semmle.label | Chi [[]] |
73-
| test.cpp:18:2:18:14 | ChiPartial | semmle.label | ChiPartial |
74-
| test.cpp:18:9:18:12 | call to rand | semmle.label | call to rand |
7541
| test.cpp:18:9:18:12 | call to rand | semmle.label | call to rand |
7642
| test.cpp:24:11:24:18 | call to get_rand | semmle.label | call to get_rand |
7743
| test.cpp:25:7:25:7 | r | semmle.label | r |
78-
| test.cpp:25:7:25:7 | r | semmle.label | r |
79-
| test.cpp:25:7:25:7 | r | semmle.label | r |
8044
| test.cpp:30:13:30:14 | Chi | semmle.label | Chi |
8145
| test.cpp:30:13:30:14 | get_rand2 output argument [[]] | semmle.label | get_rand2 output argument [[]] |
8246
| test.cpp:31:7:31:7 | r | semmle.label | r |
83-
| test.cpp:31:7:31:7 | r | semmle.label | r |
84-
| test.cpp:31:7:31:7 | r | semmle.label | r |
8547
| test.cpp:36:13:36:13 | Chi | semmle.label | Chi |
8648
| test.cpp:36:13:36:13 | get_rand3 output argument [[]] | semmle.label | get_rand3 output argument [[]] |
8749
| test.cpp:37:7:37:7 | r | semmle.label | r |
88-
| test.cpp:37:7:37:7 | r | semmle.label | r |
89-
| test.cpp:37:7:37:7 | r | semmle.label | r |
9050
#select
9151
| 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 |
9252
| 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 |
9353
| 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 |
94-
| 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 |
54+
| test.c:77:9:77:9 | r | test.c:75:13:75:19 | call to rand | 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 | call to rand | Uncontrolled value |
55+
| test.c:77:9:77:9 | r | test.c:75:13:75:19 | call to rand | 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 | call to rand | Uncontrolled value |
56+
| test.c:83:9:83:9 | r | test.c:81:14:81:17 | call to rand | test.c:83:9:83:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:81:14:81:17 | call to rand | Uncontrolled value |
57+
| test.c:83:9:83:9 | r | test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:81:23:81:26 | call to rand | Uncontrolled value |
9558
| 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 |
9659
| 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 |
9760
| test.cpp:31:7:31:7 | r | test.cpp:13:10:13:13 | call to rand | test.cpp:31:7:31:7 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:13:10:13:13 | call to rand | Uncontrolled value |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void randomTester() {
8080
{
8181
int r = (rand() ^ rand());
8282

83-
r = r - 100; // BAD [NOT DETECTED]
83+
r = r - 100; // BAD
8484
}
8585

8686
{

0 commit comments

Comments
 (0)