Skip to content

Commit 693a479

Browse files
authored
Merge branch 'main' into python-add-typetrackingnode
2 parents 1e79091 + cb1b227 commit 693a479

File tree

98 files changed

+2995
-3027
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

98 files changed

+2995
-3027
lines changed
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/Diagnostics/FailedExtractorInvocations.ql

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@
77

88
import cpp
99

10-
class AnonymousCompilation extends Compilation {
11-
override string toString() { result = "<compilation>" }
12-
}
13-
1410
string describe(Compilation c) {
1511
if c.getArgument(1) = "--mimic"
1612
then result = "compiler invocation " + concat(int i | i > 1 | c.getArgument(i), " " order by i)
@@ -19,4 +15,4 @@ string describe(Compilation c) {
1915

2016
from Compilation c
2117
where not c.normalTermination()
22-
select c, "Extraction aborted for " + describe(c), 2
18+
select "Extraction aborted for " + describe(c)

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/src/Security/CWE/CWE-190/Bounded.qll

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,6 @@ private import cpp
77
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
88
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
99

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-
2910
/**
3011
* An operand `e` of a bitwise and expression `andExpr` (i.e., `andExpr` is either an `BitwiseAndExpr`
3112
* or an `AssignAndExpr`) with operands `operand1` and `operand2` is the operand that is not `e` is upper
@@ -50,19 +31,10 @@ predicate bounded(Expr e) {
5031
) and
5132
not convertedExprMightOverflow(e)
5233
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()))
34+
// Optimitically assume that a remainder expression always yields a much smaller value.
35+
e = any(RemExpr rem).getLeftOperand()
6436
or
65-
exists(AssignRemExpr rem | boundedRem(e, rem, rem.getLValue(), rem.getRValue()))
37+
e = any(AssignRemExpr rem).getLValue()
6638
or
6739
exists(BitwiseAndExpr andExpr |
6840
boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand())
@@ -73,11 +45,11 @@ predicate bounded(Expr e) {
7345
)
7446
or
7547
// Optimitically assume that a division always yields a much smaller value.
76-
boundedDiv(e, any(DivExpr div).getLeftOperand())
48+
e = any(DivExpr div).getLeftOperand()
7749
or
78-
boundedDiv(e, any(AssignDivExpr div).getLValue())
50+
e = any(AssignDivExpr div).getLValue()
7951
or
80-
boundedDiv(e, any(RShiftExpr shift).getLeftOperand())
52+
e = any(RShiftExpr shift).getLeftOperand()
8153
or
82-
boundedDiv(e, any(AssignRShiftExpr div).getLValue())
54+
e = any(AssignRShiftExpr div).getLValue()
8355
}

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: 11 additions & 2 deletions
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
{
@@ -109,4 +109,13 @@ void randomTester() {
109109

110110
void add_100(int r) {
111111
r += 100; // GOOD
112-
}
112+
}
113+
114+
void randomTester2(int bound, int min, int max) {
115+
int r1 = rand() % bound;
116+
r1 += 100; // GOOD (`bound` may possibly be MAX_INT in which case this could
117+
// still overflow, but it's most likely fine)
118+
119+
int r2 = (rand() % (max - min + 1)) + min;
120+
r2 += 100; // GOOD (This is a common way to clamp the random value between [min, max])
121+
}

csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ public BuildAnalysis(Options options, IProgressMonitor progress)
108108
new[] { options.SolutionFile } :
109109
sourceDir.GetFiles("*.sln", SearchOption.AllDirectories).Select(d => d.FullName);
110110

111-
RestoreSolutions(solutions);
111+
if (options.UseNuGet)
112+
{
113+
RestoreSolutions(solutions);
114+
}
112115
dllDirNames.Add(packageDirectory.DirInfo.FullName);
113116
assemblyCache = new BuildAnalyser.AssemblyCache(dllDirNames, progress);
114117
AnalyseSolutions(solutions);
@@ -324,7 +327,16 @@ private void AnalyseProject(FileInfo project)
324327

325328
private void Restore(string projectOrSolution)
326329
{
327-
var exit = DotNet.RestoreToDirectory(projectOrSolution, packageDirectory.DirInfo.FullName);
330+
int exit;
331+
try
332+
{
333+
exit = DotNet.RestoreToDirectory(projectOrSolution, packageDirectory.DirInfo.FullName);
334+
}
335+
catch (FileNotFoundException)
336+
{
337+
exit = 2;
338+
}
339+
328340
switch (exit)
329341
{
330342
case 0:

0 commit comments

Comments
 (0)