Skip to content

Commit 5bfb78b

Browse files
committed
C++: Block flow through all bitwise 'and' and 'or' operations. This seems to be a common source of false positives on LGTM.
1 parent e8bba78 commit 5bfb78b

File tree

3 files changed

+11
-17
lines changed

3 files changed

+11
-17
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,15 @@ class UncontrolledArithConfiguration extends TaintTracking::Configuration {
9797

9898
override predicate isSink(DataFlow::Node sink) { missingGuard(sink.asExpr(), _) }
9999

100-
override predicate isSanitizer(DataFlow::Node barrier) { bounded(barrier.asExpr()) }
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(BinaryBitwiseOperation op | op instanceof BitwiseOrExpr or op instanceof BitwiseAndExpr)
107+
.getAnOperand*()
108+
}
101109
}
102110

103111
Expr getExpr(DataFlow::Node node) { result = [node.asExpr(), node.asDefiningArgument()] }

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@ edges
22
| test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r |
33
| test.c:34:13:34:18 | call to rand | test.c:35:5:35:5 | r |
44
| test.c:44:13:44:16 | call to rand | test.c:45:5:45: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 |
95
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
106
| test.cpp:8:9:8:12 | Store | test.cpp:24:11:24:18 | call to get_rand |
117
| test.cpp:8:9:8:12 | call to rand | test.cpp:8:9:8:12 | Store |
@@ -25,12 +21,6 @@ nodes
2521
| test.c:35:5:35:5 | r | semmle.label | r |
2622
| test.c:44:13:44:16 | call to rand | semmle.label | call to rand |
2723
| test.c:45:5:45:5 | 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 |
30-
| 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 |
3424
| test.c:99:14:99:19 | call to rand | semmle.label | call to rand |
3525
| test.c:100:5:100:5 | r | semmle.label | r |
3626
| test.cpp:8:9:8:12 | Store | semmle.label | Store |
@@ -51,10 +41,6 @@ nodes
5141
| 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 |
5242
| 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 |
5343
| 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 |
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 |
5844
| 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 |
5945
| 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 |
6046
| 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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ void randomTester() {
7474
{
7575
int r = RAND2();
7676

77-
r = r - 100; // BAD
77+
r = r - 100; // BAD [NOT DETECTED]
7878
}
7979

8080
{
8181
int r = (rand() ^ rand());
8282

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

8686
{

0 commit comments

Comments
 (0)