Skip to content

Commit 697e2e2

Browse files
lcarteylcartey
committed
IntegerOverflow: Expand supported operations
 * Support unary operations (unary plus/minus) * Support arithmetic assign operations
1 parent 57cf6ec commit 697e2e2

File tree

8 files changed

+122
-72
lines changed

8 files changed

+122
-72
lines changed

‎c/cert/src/rules/INT30-C/UnsignedIntegerOperationsWrapAround.ql

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,19 @@ import codingstandards.cpp.Overflow
1919
import semmle.code.cpp.controlflow.Guards
2020
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
2121

22-
/* TODO: review the table to restrict to only those operations that actually overflow */
23-
from InterestingBinaryOverflowingExpr bop
22+
from InterestingOverflowingOperation op
2423
where
25-
not isExcluded(bop, IntegerOverflowPackage::unsignedIntegerOperationsWrapAroundQuery()) and
26-
bop.getType().getUnderlyingType().(IntegralType).isUnsigned() and
24+
not isExcluded(op, IntegerOverflowPackage::unsignedIntegerOperationsWrapAroundQuery()) and
25+
op.getType().getUnderlyingType().(IntegralType).isUnsigned() and
2726
// Not within a guard condition
28-
not exists(GuardCondition gc | gc.getAChild*() = bop) and
27+
not exists(GuardCondition gc | gc.getAChild*() = op) and
2928
// Not guarded by a check, where the check is not an invalid overflow check
30-
not bop.getAGuardingGVN() = globalValueNumber(bop.getAChild*()) and
29+
not op.getAGuardingGVN() = globalValueNumber(op.getAChild*()) and
3130
// Is not checked after the operation
32-
not bop.hasValidPostCheck()
33-
select bop,
34-
"Binary expression ..." + bop.getOperator() + "... of type " + bop.getType().getUnderlyingType() +
35-
" may wrap."
31+
not op.hasValidPostCheck() and
32+
// Permitted by exception 3
33+
not op instanceof LShiftExpr and
34+
// Permitted by exception 2 - zero case is handled in separate query
35+
not op instanceof DivExpr
36+
select op,
37+
"Operation " + op.getOperator() + " of type " + op.getType().getUnderlyingType() + " may wrap."

‎c/cert/src/rules/INT32-C/SignedIntegerOverflow.ql

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@ import codingstandards.cpp.Overflow
1818
import semmle.code.cpp.controlflow.Guards
1919
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
2020

21-
/* TODO: review the table to restrict to only those operations that actually overflow */
22-
from InterestingBinaryOverflowingExpr bop
21+
from InterestingOverflowingOperation op
2322
where
24-
not isExcluded(bop, IntegerOverflowPackage::signedIntegerOverflowQuery()) and
25-
bop.getType().getUnderlyingType().(IntegralType).isSigned() and
23+
not isExcluded(op, IntegerOverflowPackage::signedIntegerOverflowQuery()) and
24+
op.getType().getUnderlyingType().(IntegralType).isSigned() and
2625
// Not checked before the operation
27-
not bop.hasValidPreCheck() and
26+
not op.hasValidPreCheck() and
2827
// Not guarded by a check, where the check is not an invalid overflow check
29-
not bop.getAGuardingGVN() = globalValueNumber(bop.getAChild*())
30-
select bop,
31-
"Binary expression ..." + bop.getOperator() + "... of type " + bop.getType().getUnderlyingType() +
28+
not op.getAGuardingGVN() = globalValueNumber(op.getAChild*())
29+
select op,
30+
"Operation " + op.getOperator() + " of type " + op.getType().getUnderlyingType() +
3231
" may overflow or underflow."

‎c/cert/test/rules/INT30-C/UnsignedIntegerOperationsWrapAround.expected

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1-
| test.c:4:3:4:9 | ... + ... | Binary expression ...+... of type unsigned int may wrap. |
2-
| test.c:48:3:48:9 | ... - ... | Binary expression ...-... of type unsigned int may wrap. |
1+
| test.c:4:3:4:9 | ... + ... | Operation + of type unsigned int may wrap. |
2+
| test.c:5:3:5:10 | ... += ... | Operation += of type unsigned int may wrap. |
3+
| test.c:58:3:58:9 | ... - ... | Operation - of type unsigned int may wrap. |
4+
| test.c:59:3:59:10 | ... -= ... | Operation -= of type unsigned int may wrap. |

‎c/cert/test/rules/INT30-C/test.c

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,25 @@
11
#include <limits.h>
22

33
void test_add_simple(unsigned int i1, unsigned int i2) {
4-
i1 + i2; // NON_COMPLIANT - not bounds checked
4+
i1 + i2; // NON_COMPLIANT - not bounds checked
5+
i1 += i2; // NON_COMPLIANT - not bounds checked
56
}
67

78
void test_add_precheck(unsigned int i1, unsigned int i2) {
89
if (UINT_MAX - i1 < i2) {
910
// handle error
1011
} else {
11-
i1 + i2; // COMPLIANT - bounds checked
12+
i1 + i2; // COMPLIANT - bounds checked
13+
i1 += i2; // COMPLIANT - bounds checked
1214
}
1315
}
1416

1517
void test_add_precheck_2(unsigned int i1, unsigned int i2) {
1618
if (i1 + i2 < i1) {
1719
// handle error
1820
} else {
19-
i1 + i2; // COMPLIANT - bounds checked
21+
i1 + i2; // COMPLIANT - bounds checked
22+
i1 += i2; // COMPLIANT - bounds checked
2023
}
2124
}
2225

@@ -25,16 +28,23 @@ void test_add_postcheck(unsigned int i1, unsigned int i2) {
2528
if (i3 < i1) {
2629
// handle error
2730
}
31+
i1 += i2; // COMPLIANT - checked for overflow afterwards
32+
if (i1 < i2) {
33+
// handle error
34+
}
2835
}
2936

3037
void test_ex2(unsigned int i1, unsigned int i2) {
3138
unsigned int ci1 = 2;
3239
unsigned int ci2 = 3;
3340
ci1 + ci2; // COMPLIANT, compile time constants
3441
i1 + 0; // COMPLIANT
42+
i1 += 0; // COMPLIANT
3543
i1 - 0; // COMPLIANT
44+
i1 -= 0; // COMPLIANT
3645
UINT_MAX - i1; // COMPLIANT - cannot be smaller than 0
3746
i1 * 1; // COMPLIANT
47+
i1 *= 1; // COMPLIANT
3848
if (0 <= i1 && i1 < 32) {
3949
UINT_MAX >> i1; // COMPLIANT
4050
}
@@ -45,14 +55,16 @@ void test_ex3(unsigned int i1, unsigned int i2) {
4555
}
4656

4757
void test_sub_simple(unsigned int i1, unsigned int i2) {
48-
i1 - i2; // NON_COMPLIANT - not bounds checked
58+
i1 - i2; // NON_COMPLIANT - not bounds checked
59+
i1 -= i2; // NON_COMPLIANT - not bounds checked
4960
}
5061

5162
void test_sub_precheck(unsigned int i1, unsigned int i2) {
5263
if (i1 < i2) {
5364
// handle error
5465
} else {
55-
i1 - i2; // COMPLIANT - bounds checked
66+
i1 - i2; // COMPLIANT - bounds checked
67+
i1 -= i2; // COMPLIANT - bounds checked
5668
}
5769
}
5870

@@ -61,4 +73,8 @@ void test_sub_postcheck(unsigned int i1, unsigned int i2) {
6173
if (i3 > i1) {
6274
// handle error
6375
}
76+
i1 -= i2; // COMPLIANT - checked for wrap afterwards
77+
if (i1 > i2) {
78+
// handle error
79+
}
6480
}

‎c/cert/test/rules/INT32-C/SignedIntegerOverflow.expected

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
1-
| test.c:6:3:6:9 | ... + ... | Binary expression ...+... of type int may overflow or underflow. |
2-
| test.c:20:7:20:13 | ... + ... | Binary expression ...+... of type int may overflow or underflow. |
3-
| test.c:23:5:23:11 | ... + ... | Binary expression ...+... of type int may overflow or underflow. |
4-
| test.c:28:19:28:25 | ... + ... | Binary expression ...+... of type int may overflow or underflow. |
5-
| test.c:36:3:36:9 | ... - ... | Binary expression ...-... of type int may overflow or underflow. |
6-
| test.c:49:19:49:25 | ... - ... | Binary expression ...-... of type int may overflow or underflow. |
7-
| test.c:56:3:56:8 | ... * ... | Binary expression ...*... of type int may overflow or underflow. |
1+
| test.c:6:3:6:9 | ... + ... | Operation + of type int may overflow or underflow. |
2+
| test.c:7:3:7:10 | ... += ... | Operation += of type signed int may overflow or underflow. |
3+
| test.c:22:7:22:13 | ... + ... | Operation + of type int may overflow or underflow. |
4+
| test.c:25:5:25:11 | ... + ... | Operation + of type int may overflow or underflow. |
5+
| test.c:26:5:26:12 | ... += ... | Operation += of type signed int may overflow or underflow. |
6+
| test.c:31:19:31:25 | ... + ... | Operation + of type int may overflow or underflow. |
7+
| test.c:36:3:36:10 | ... += ... | Operation += of type signed int may overflow or underflow. |
8+
| test.c:43:3:43:9 | ... - ... | Operation - of type int may overflow or underflow. |
9+
| test.c:44:3:44:10 | ... -= ... | Operation -= of type signed int may overflow or underflow. |
10+
| test.c:58:19:58:25 | ... - ... | Operation - of type int may overflow or underflow. |
11+
| test.c:62:3:62:10 | ... -= ... | Operation -= of type signed int may overflow or underflow. |
12+
| test.c:69:3:69:8 | ... * ... | Operation * of type int may overflow or underflow. |
13+
| test.c:70:3:70:10 | ... *= ... | Operation *= of type signed int may overflow or underflow. |
14+
| test.c:153:3:153:10 | ... << ... | Operation << of type signed int may overflow or underflow. |
15+
| test.c:154:3:154:11 | ... <<= ... | Operation <<= of type signed int may overflow or underflow. |
16+
| test.c:173:3:173:5 | - ... | Operation - of type signed int may overflow or underflow. |

‎c/cert/test/rules/INT32-C/test.c

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
#include <stdint.h>
44

55
void test_add_simple(signed int i1, signed int i2) {
6-
i1 + i2; // NON_COMPLIANT - not bounds checked
6+
i1 + i2; // NON_COMPLIANT - not bounds checked
7+
i1 += i2; // NON_COMPLIANT - not bounds checked
78
}
89

910
void test_add_precheck(signed int i1, signed int i2) {
@@ -12,15 +13,17 @@ void test_add_precheck(signed int i1, signed int i2) {
1213
((i2 < 0) && (i1 < (INT_MIN - i2)))) {
1314
// handle error
1415
} else {
15-
i1 + i2; // COMPLIANT - bounds appropriately checked
16+
i1 + i2; // COMPLIANT - bounds appropriately checked
17+
i1 += i2; // COMPLIANT - bounds appropriately checked
1618
}
1719
}
1820

1921
void test_add_precheck_2(signed int i1, signed int i2) {
2022
if (i1 + i2 < i1) { // NON_COMPLIANT - bad overflow check - undefined behavior
2123
// handle error
2224
} else {
23-
i1 + i2; // NON_COMPLIANT
25+
i1 + i2; // NON_COMPLIANT
26+
i1 += i2; // NON_COMPLIANT
2427
}
2528
}
2629

@@ -30,18 +33,24 @@ void test_add_postcheck(signed int i1, signed int i2) {
3033
if (i3 < i1) {
3134
// handle error
3235
}
36+
i1 += i2; // NON_COMPLIANT
37+
if (i1 < i2) {
38+
// handle error
39+
}
3340
}
3441

3542
void test_sub_simple(signed int i1, signed int i2) {
36-
i1 - i2; // NON_COMPLIANT - not bounds checked
43+
i1 - i2; // NON_COMPLIANT - not bounds checked
44+
i1 -= i2; // NON_COMPLIANT - not bounds checked
3745
}
3846

3947
void test_sub_precheck(signed int i1, signed int i2) {
4048
// Style recomended by CERT
4149
if ((i2 > 0 && i1 < INT_MIN + i2) || (i2 < 0 && i1 > INT_MAX + i2)) {
4250
// handle error
4351
} else {
44-
i1 - i2; // COMPLIANT - bounds checked
52+
i1 - i2; // COMPLIANT - bounds checked
53+
i1 -= i2; // COMPLIANT - bounds checked
4554
}
4655
}
4756

@@ -50,10 +59,15 @@ void test_sub_postcheck(signed int i1, signed int i2) {
5059
if (i3 > i1) {
5160
// handle error
5261
}
62+
i1 -= i2; // NON_COMPLIANT - underflow is undefined behavior.
63+
if (i1 > i2) {
64+
// handle error
65+
}
5366
}
5467

5568
void test_mul_simple(signed int i1, signed int i2) {
56-
i1 *i2; // NON_COMPLIANT
69+
i1 *i2; // NON_COMPLIANT
70+
i1 *= i2; // NON_COMPLIANT
5771
}
5872

5973
void test_mul_precheck(signed int i1, signed int i2) {
@@ -66,6 +80,7 @@ void test_mul_precheck(signed int i1, signed int i2) {
6680
} else {
6781
i1 *i2; // COMPLIANT - checked
6882
result = (signed int)tmp;
83+
i1 *= i2; // COMPLIANT - checked
6984
}
7085
}
7186

@@ -94,43 +109,49 @@ void test_mul_precheck_2(signed int i1, signed int i2) {
94109
}
95110
}
96111
}
97-
i1 *i2; // COMPLIANT
112+
i1 *i2; // COMPLIANT
113+
i1 *= i2; // COMPLIANT
98114
}
99115

100116
void test_simple_div(signed int i1, signed int i2) {
101117
if (i2 == 0) {
102118
// handle error
103119
} else {
104-
i1 / i2; // NON_COMPLIANT
120+
i1 / i2; // NON_COMPLIANT
121+
i1 /= i2; // NON_COMPLIANT
105122
}
106123
}
107124

108125
void test_div_precheck(signed int i1, signed int i2) {
109126
if ((i2 == 0) || ((i1 == LONG_MIN) && (i2 == -1))) {
110127
/* Handle error */
111128
} else {
112-
i1 / i2; // COMPLIANT
129+
i1 / i2; // COMPLIANT
130+
i1 /= i2; // COMPLIANT
113131
}
114132
}
115133

116134
void test_simple_rem(signed int i1, signed int i2) {
117135
if (i2 == 0) {
118136
// handle error
119137
} else {
120-
i1 % i2; // NON_COMPLIANT
138+
i1 % i2; // NON_COMPLIANT
139+
i1 %= i2; // NON_COMPLIANT
121140
}
122141
}
123142

124143
void test_rem_precheck(signed int i1, signed int i2) {
125144
if ((i2 == 0) || ((i1 == LONG_MIN) && (i2 == -1))) {
126145
/* Handle error */
127146
} else {
128-
i1 % i2; // COMPLIANT
147+
i1 % i2; // COMPLIANT
148+
i1 %= i2; // COMPLIANT
129149
}
130150
}
131151

132152
void test_simple_left_shift(signed int i1, signed int i2) {
133-
i1 << i2; // NON_COMPLIANT
153+
i1 << i2; // NON_COMPLIANT
154+
i1 <<= i2; // NON_COMPLIANT
134155
}
135156

136157
/* Returns the number of set bits */
@@ -143,7 +164,8 @@ void test_left_shift_precheck(signed int i1, signed int i2) {
143164
(i1 > (INT_MAX >> i2))) {
144165
// handle error
145166
} else {
146-
i1 << i2; // COMPLIANT
167+
i1 << i2; // COMPLIANT
168+
i1 <<= i2; // COMPLIANT
147169
}
148170
}
149171

‎cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ import codingstandards.cpp.Overflow
1919
import semmle.code.cpp.controlflow.Guards
2020
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
2121

22-
from InterestingBinaryOverflowingExpr e
22+
from InterestingOverflowingOperation e
2323
where
2424
not isExcluded(e, IntegerConversionPackage::integerExpressionLeadToDataLossQuery()) and
2525
// Not within a guard condition
2626
not exists(GuardCondition gc | gc.getAChild*() = e) and
2727
// Not guarded by a check, where the check is not an invalid overflow check
2828
not e.getAGuardingGVN() = globalValueNumber(e.getAChild*()) and
2929
// Covered by `IntMultToLong.ql` instead
30-
not e instanceof MulExpr
30+
not e instanceof MulExpr and
31+
// Not covered by this query - overflow/underflow in division is rare
32+
not e instanceof DivExpr
3133
select e, "Binary expression ..." + e.getOperator() + "... may overflow."

0 commit comments

Comments
 (0)