Skip to content

Commit b7206c1

Browse files
authored
Merge pull request github#6581 from geoffw0/uncontrolledarith2
CPP: Improvements for cpp/uncontrolled-arithmetic
2 parents 138a7ae + d6368c3 commit b7206c1

File tree

4 files changed

+164
-31
lines changed

4 files changed

+164
-31
lines changed

cpp/ql/lib/semmle/code/cpp/security/Overflow.qll

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ private predicate stmtDominates(Stmt dominator, Stmt dominated) {
4141
}
4242

4343
/**
44-
* Holds if the value of `use` is guarded to be less than something.
44+
* Holds if the value of `use` is guarded to be less than something, and `e`
45+
* is in code controlled by that guard (where the guard condition held).
4546
*/
4647
pragma[nomagic]
4748
predicate guardedLesser(Operation e, Expr use) {
@@ -67,7 +68,8 @@ predicate guardedLesser(Operation e, Expr use) {
6768
}
6869

6970
/**
70-
* Holds if the value of `use` is guarded to be greater than something.
71+
* Holds if the value of `use` is guarded to be greater than something, and `e`
72+
* is in code controlled by that guard (where the guard condition held).
7173
*/
7274
pragma[nomagic]
7375
predicate guardedGreater(Operation e, Expr use) {
@@ -120,6 +122,10 @@ predicate missingGuardAgainstOverflow(Operation e, VariableAccess use) {
120122
// overflow possible if large or small
121123
e instanceof MulExpr and
122124
not (guardedLesser(e, varUse(v)) and guardedGreater(e, varUse(v)))
125+
or
126+
// overflow possible if large or small
127+
e instanceof AssignMulExpr and
128+
not (guardedLesser(e, varUse(v)) and guardedGreater(e, varUse(v)))
123129
)
124130
}
125131

@@ -147,5 +153,9 @@ predicate missingGuardAgainstUnderflow(Operation e, VariableAccess use) {
147153
// underflow possible if large or small
148154
e instanceof MulExpr and
149155
not (guardedLesser(e, varUse(v)) and guardedGreater(e, varUse(v)))
156+
or
157+
// underflow possible if large or small
158+
e instanceof AssignMulExpr and
159+
not (guardedLesser(e, varUse(v)) and guardedGreater(e, varUse(v)))
150160
)
151161
}

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

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ edges
88
| test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r |
99
| test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r |
1010
| test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r |
11-
| test.c:148:22:148:25 | call to rand | test.c:150:9:150:9 | r |
12-
| test.c:148:22:148:27 | (unsigned int)... | test.c:150:9:150:9 | r |
11+
| test.c:131:13:131:16 | call to rand | test.c:133:5:133:5 | r |
12+
| test.c:137:13:137:16 | call to rand | test.c:139:10:139:10 | r |
13+
| test.c:155:22:155:25 | call to rand | test.c:157:9:157:9 | r |
14+
| test.c:155:22:155:27 | (unsigned int)... | test.c:157:9:157:9 | r |
1315
| test.cpp:8:9:8:12 | Store | test.cpp:24:11:24:18 | call to get_rand |
1416
| test.cpp:8:9:8:12 | call to rand | test.cpp:8:9:8:12 | Store |
1517
| test.cpp:13:2:13:15 | Chi [[]] | test.cpp:30:13:30:14 | get_rand2 output argument [[]] |
@@ -21,11 +23,23 @@ edges
2123
| test.cpp:30:13:30:14 | get_rand2 output argument [[]] | test.cpp:30:13:30:14 | Chi |
2224
| test.cpp:36:13:36:13 | Chi | test.cpp:37:7:37:7 | r |
2325
| test.cpp:36:13:36:13 | get_rand3 output argument [[]] | test.cpp:36:13:36:13 | Chi |
24-
| test.cpp:78:10:78:13 | call to rand | test.cpp:82:10:82:10 | x |
25-
| test.cpp:90:10:90:13 | call to rand | test.cpp:94:10:94:10 | x |
26-
| test.cpp:129:10:129:13 | call to rand | test.cpp:132:10:132:10 | b |
27-
| test.cpp:147:11:147:14 | call to rand | test.cpp:149:11:149:16 | (int)... |
28-
| test.cpp:147:11:147:14 | call to rand | test.cpp:149:16:149:16 | y |
26+
| test.cpp:62:19:62:22 | call to rand | test.cpp:65:9:65:9 | x |
27+
| test.cpp:62:19:62:24 | (unsigned int)... | test.cpp:65:9:65:9 | x |
28+
| test.cpp:86:10:86:13 | call to rand | test.cpp:90:10:90:10 | x |
29+
| test.cpp:98:10:98:13 | call to rand | test.cpp:102:10:102:10 | x |
30+
| test.cpp:137:10:137:13 | call to rand | test.cpp:146:9:146:9 | y |
31+
| test.cpp:151:10:151:13 | call to rand | test.cpp:154:10:154:10 | b |
32+
| test.cpp:169:11:169:14 | call to rand | test.cpp:171:11:171:16 | (int)... |
33+
| test.cpp:169:11:169:14 | call to rand | test.cpp:171:16:171:16 | y |
34+
| test.cpp:189:10:189:13 | call to rand | test.cpp:196:7:196:7 | x |
35+
| test.cpp:189:10:189:13 | call to rand | test.cpp:198:7:198:7 | x |
36+
| test.cpp:189:10:189:13 | call to rand | test.cpp:199:7:199:7 | x |
37+
| test.cpp:190:10:190:13 | call to rand | test.cpp:204:7:204:7 | y |
38+
| test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y |
39+
| test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y |
40+
| test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x |
41+
| test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x |
42+
| test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x |
2943
nodes
3044
| test.c:18:13:18:16 | call to rand | semmle.label | call to rand |
3145
| test.c:21:17:21:17 | r | semmle.label | r |
@@ -43,9 +57,13 @@ nodes
4357
| test.c:100:5:100:5 | r | semmle.label | r |
4458
| test.c:125:13:125:16 | call to rand | semmle.label | call to rand |
4559
| test.c:127:9:127:9 | r | semmle.label | r |
46-
| test.c:148:22:148:25 | call to rand | semmle.label | call to rand |
47-
| test.c:148:22:148:27 | (unsigned int)... | semmle.label | (unsigned int)... |
48-
| test.c:150:9:150:9 | r | semmle.label | r |
60+
| test.c:131:13:131:16 | call to rand | semmle.label | call to rand |
61+
| test.c:133:5:133:5 | r | semmle.label | r |
62+
| test.c:137:13:137:16 | call to rand | semmle.label | call to rand |
63+
| test.c:139:10:139:10 | r | semmle.label | r |
64+
| test.c:155:22:155:25 | call to rand | semmle.label | call to rand |
65+
| test.c:155:22:155:27 | (unsigned int)... | semmle.label | (unsigned int)... |
66+
| test.c:157:9:157:9 | r | semmle.label | r |
4967
| test.cpp:8:9:8:12 | Store | semmle.label | Store |
5068
| test.cpp:8:9:8:12 | call to rand | semmle.label | call to rand |
5169
| test.cpp:13:2:13:15 | Chi [[]] | semmle.label | Chi [[]] |
@@ -60,15 +78,33 @@ nodes
6078
| test.cpp:36:13:36:13 | Chi | semmle.label | Chi |
6179
| test.cpp:36:13:36:13 | get_rand3 output argument [[]] | semmle.label | get_rand3 output argument [[]] |
6280
| test.cpp:37:7:37:7 | r | semmle.label | r |
63-
| test.cpp:78:10:78:13 | call to rand | semmle.label | call to rand |
64-
| test.cpp:82:10:82:10 | x | semmle.label | x |
65-
| test.cpp:90:10:90:13 | call to rand | semmle.label | call to rand |
66-
| test.cpp:94:10:94:10 | x | semmle.label | x |
67-
| test.cpp:129:10:129:13 | call to rand | semmle.label | call to rand |
68-
| test.cpp:132:10:132:10 | b | semmle.label | b |
69-
| test.cpp:147:11:147:14 | call to rand | semmle.label | call to rand |
70-
| test.cpp:149:11:149:16 | (int)... | semmle.label | (int)... |
71-
| test.cpp:149:16:149:16 | y | semmle.label | y |
81+
| test.cpp:62:19:62:22 | call to rand | semmle.label | call to rand |
82+
| test.cpp:62:19:62:24 | (unsigned int)... | semmle.label | (unsigned int)... |
83+
| test.cpp:65:9:65:9 | x | semmle.label | x |
84+
| test.cpp:86:10:86:13 | call to rand | semmle.label | call to rand |
85+
| test.cpp:90:10:90:10 | x | semmle.label | x |
86+
| test.cpp:98:10:98:13 | call to rand | semmle.label | call to rand |
87+
| test.cpp:102:10:102:10 | x | semmle.label | x |
88+
| test.cpp:137:10:137:13 | call to rand | semmle.label | call to rand |
89+
| test.cpp:146:9:146:9 | y | semmle.label | y |
90+
| test.cpp:151:10:151:13 | call to rand | semmle.label | call to rand |
91+
| test.cpp:154:10:154:10 | b | semmle.label | b |
92+
| test.cpp:169:11:169:14 | call to rand | semmle.label | call to rand |
93+
| test.cpp:171:11:171:16 | (int)... | semmle.label | (int)... |
94+
| test.cpp:171:16:171:16 | y | semmle.label | y |
95+
| test.cpp:189:10:189:13 | call to rand | semmle.label | call to rand |
96+
| test.cpp:190:10:190:13 | call to rand | semmle.label | call to rand |
97+
| test.cpp:196:7:196:7 | x | semmle.label | x |
98+
| test.cpp:198:7:198:7 | x | semmle.label | x |
99+
| test.cpp:199:7:199:7 | x | semmle.label | x |
100+
| test.cpp:204:7:204:7 | y | semmle.label | y |
101+
| test.cpp:205:7:205:7 | y | semmle.label | y |
102+
| test.cpp:208:7:208:7 | y | semmle.label | y |
103+
| test.cpp:215:11:215:14 | call to rand | semmle.label | call to rand |
104+
| test.cpp:219:8:219:8 | x | semmle.label | x |
105+
| test.cpp:223:20:223:23 | call to rand | semmle.label | call to rand |
106+
| test.cpp:223:20:223:25 | (unsigned int)... | semmle.label | (unsigned int)... |
107+
| test.cpp:227:8:227:8 | x | semmle.label | x |
72108
#select
73109
| 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 |
74110
| 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 |
@@ -79,13 +115,27 @@ nodes
79115
| 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 overflow. | test.c:81:23:81:26 | call to rand | Uncontrolled value |
80116
| 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 overflow. | test.c:99:14:99:19 | call to rand | Uncontrolled value |
81117
| test.c:127:9:127:9 | r | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:125:13:125:16 | call to rand | Uncontrolled value |
82-
| test.c:150:9:150:9 | r | test.c:148:22:148:25 | call to rand | test.c:150:9:150:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:148:22:148:25 | call to rand | Uncontrolled value |
83-
| test.c:150:9:150:9 | r | test.c:148:22:148:27 | (unsigned int)... | test.c:150:9:150:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:148:22:148:25 | call to rand | Uncontrolled value |
118+
| test.c:133:5:133:5 | r | test.c:131:13:131:16 | call to rand | test.c:133:5:133:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:131:13:131:16 | call to rand | Uncontrolled value |
119+
| test.c:139:10:139:10 | r | test.c:137:13:137:16 | call to rand | test.c:139:10:139:10 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:137:13:137:16 | call to rand | Uncontrolled value |
120+
| test.c:157:9:157:9 | r | test.c:155:22:155:25 | call to rand | test.c:157:9:157:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:155:22:155:25 | call to rand | Uncontrolled value |
121+
| test.c:157:9:157:9 | r | test.c:155:22:155:27 | (unsigned int)... | test.c:157:9:157:9 | r | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:155:22:155:25 | call to rand | Uncontrolled value |
84122
| 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 |
85123
| 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 |
86124
| test.cpp:37:7:37:7 | r | test.cpp:18:9:18:12 | call to rand | test.cpp:37:7:37:7 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:18:9:18:12 | call to rand | Uncontrolled value |
87-
| test.cpp:82:10:82:10 | x | test.cpp:78:10:78:13 | call to rand | test.cpp:82:10:82:10 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:78:10:78:13 | call to rand | Uncontrolled value |
88-
| test.cpp:94:10:94:10 | x | test.cpp:90:10:90:13 | call to rand | test.cpp:94:10:94:10 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:90:10:90:13 | call to rand | Uncontrolled value |
89-
| test.cpp:132:10:132:10 | b | test.cpp:129:10:129:13 | call to rand | test.cpp:132:10:132:10 | b | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:129:10:129:13 | call to rand | Uncontrolled value |
90-
| test.cpp:149:11:149:16 | (int)... | test.cpp:147:11:147:14 | call to rand | test.cpp:149:11:149:16 | (int)... | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:147:11:147:14 | call to rand | Uncontrolled value |
91-
| test.cpp:149:16:149:16 | y | test.cpp:147:11:147:14 | call to rand | test.cpp:149:16:149:16 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:147:11:147:14 | call to rand | Uncontrolled value |
125+
| test.cpp:65:9:65:9 | x | test.cpp:62:19:62:22 | call to rand | test.cpp:65:9:65:9 | x | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.cpp:62:19:62:22 | call to rand | Uncontrolled value |
126+
| test.cpp:65:9:65:9 | x | test.cpp:62:19:62:24 | (unsigned int)... | test.cpp:65:9:65:9 | x | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.cpp:62:19:62:22 | call to rand | Uncontrolled value |
127+
| test.cpp:90:10:90:10 | x | test.cpp:86:10:86:13 | call to rand | test.cpp:90:10:90:10 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:86:10:86:13 | call to rand | Uncontrolled value |
128+
| test.cpp:102:10:102:10 | x | test.cpp:98:10:98:13 | call to rand | test.cpp:102:10:102:10 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:98:10:98:13 | call to rand | Uncontrolled value |
129+
| test.cpp:146:9:146:9 | y | test.cpp:137:10:137:13 | call to rand | test.cpp:146:9:146:9 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:137:10:137:13 | call to rand | Uncontrolled value |
130+
| test.cpp:154:10:154:10 | b | test.cpp:151:10:151:13 | call to rand | test.cpp:154:10:154:10 | b | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:151:10:151:13 | call to rand | Uncontrolled value |
131+
| test.cpp:171:11:171:16 | (int)... | test.cpp:169:11:169:14 | call to rand | test.cpp:171:11:171:16 | (int)... | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:169:11:169:14 | call to rand | Uncontrolled value |
132+
| test.cpp:171:16:171:16 | y | test.cpp:169:11:169:14 | call to rand | test.cpp:171:16:171:16 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:169:11:169:14 | call to rand | Uncontrolled value |
133+
| test.cpp:196:7:196:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:196:7:196:7 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | Uncontrolled value |
134+
| test.cpp:198:7:198:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:198:7:198:7 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | Uncontrolled value |
135+
| test.cpp:199:7:199:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:199:7:199:7 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | Uncontrolled value |
136+
| test.cpp:204:7:204:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:204:7:204:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
137+
| test.cpp:205:7:205:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
138+
| test.cpp:208:7:208:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
139+
| test.cpp:219:8:219:8 | x | test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:215:11:215:14 | call to rand | Uncontrolled value |
140+
| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |
141+
| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,17 @@ void moreTests() {
126126

127127
r = r * 100; // BAD
128128
}
129+
129130
{
130131
int r = rand();
131132

132-
r *= 100; // BAD [NOT DETECTED]
133+
r *= 100; // BAD
134+
}
135+
136+
{
137+
int r = rand();
138+
int v = 100;
139+
v *= r; // BAD
133140
}
134141

135142
{

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

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ int rand(int min, int max);
4242
unsigned rand(int max);
4343

4444
void test_with_bounded_randomness() {
45-
int r = rand(0, 10);
46-
r++; // GOOD
45+
int r = rand(0, 10);
46+
r++; // GOOD
4747

4848
unsigned unsigned_r = rand(10);
4949
unsigned_r++; // GOOD
@@ -57,6 +57,14 @@ int test_remainder_subtract()
5757
return x - y; // GOOD (as y <= x)
5858
}
5959

60+
unsigned int test_remainder_subtract_unsigned()
61+
{
62+
unsigned int x = rand();
63+
unsigned int y = x % 100; // y <= x
64+
65+
return x - y; // GOOD (as y <= x) [FALSE POSITIVE]
66+
}
67+
6068
typedef unsigned long size_t;
6169
int snprintf(char *s, size_t n, const char *format, ...);
6270

@@ -124,6 +132,20 @@ int test_conditional_assignment_2()
124132
return y * 10; // GOOD (as y <= 100)
125133
}
126134

135+
int test_conditional_assignment_3()
136+
{
137+
int x = rand();
138+
int y = 100;
139+
int c = 10;
140+
141+
if (x < y)
142+
{
143+
y = x;
144+
}
145+
146+
return y * c; // GOOD (as y <= 100) [FALSE POSITIVE]
147+
}
148+
127149
int test_underflow()
128150
{
129151
int x = rand();
@@ -161,3 +183,47 @@ void test_float()
161183
int z = (int)y * 5; // GOOD
162184
}
163185
}
186+
187+
void test_if_const_bounded()
188+
{
189+
int x = rand();
190+
int y = rand();
191+
int c = 10;
192+
193+
if (x < 1000)
194+
{
195+
x = x * 2; // GOOD
196+
x = x * c; // GOOD [FALSE POSITIVE]
197+
} else {
198+
x = x * 2; // BAD
199+
x = x * c; // BAD
200+
}
201+
202+
if (y > 1000)
203+
{
204+
y = y * 2; // BAD
205+
y = y * c; // BAD
206+
} else {
207+
y = y * 2; // GOOD
208+
y = y * c; // GOOD [FALSE POSITIVE]
209+
}
210+
}
211+
212+
void test_mod_limit()
213+
{
214+
{
215+
int x = rand();
216+
int y = 100;
217+
int z;
218+
219+
z = (x + y) % 1000; // BAD
220+
}
221+
222+
{
223+
unsigned int x = rand();
224+
unsigned int y = 100;
225+
unsigned int z;
226+
227+
z = (x + y) % 1000; // DUBIOUS (this could overflow but the result is controlled) [REPORTED]
228+
}
229+
}

0 commit comments

Comments
 (0)