Skip to content

Commit 2aca40a

Browse files
jbjjbj
authored
Merge pull request #736 from geoffw0/macroinv2
CPP: Deprecate MacroInvocationExpr and MacroInvocationStmt
2 parents 281c944 + 6088ca5 commit 2aca40a

File tree

7 files changed

+170
-21
lines changed

7 files changed

+170
-21
lines changed

‎cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ predicate stringArray(Variable arr, AggregateLiteral init) {
5555
// overwrite some of them with untrusted data.
5656
}
5757

58-
predicate underscoreMacro(MacroInvocationExpr e) {
59-
e.getMacroName() = "_"
58+
predicate underscoreMacro(Expr e) {
59+
exists(MacroInvocation mi |
60+
mi.getMacroName() = "_" and
61+
mi.getExpr() = e
62+
)
6063
}
6164

6265
/**

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import semmle.code.cpp.security.TaintTracking
1818

1919
predicate isRandValue(Expr e) {
2020
e.(FunctionCall).getTarget().getName() = "rand" or
21-
exists(FunctionCall fc |
22-
fc = e.(MacroInvocationExpr).getInvocation().getExpr().getAChild*()
23-
| fc.getTarget().getName() = "rand"
21+
exists(MacroInvocation mi |
22+
e = mi.getExpr() and
23+
e.getAChild*().(FunctionCall).getTarget().getName() = "rand"
2424
)
2525
}
2626

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

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,29 @@ import semmle.code.cpp.security.Overflow
1818
import semmle.code.cpp.security.Security
1919
import semmle.code.cpp.security.TaintTracking
2020

21-
predicate isMaxValue(MacroInvocationExpr mie) {
22-
mie.getMacroName() = "CHAR_MAX" or
23-
mie.getMacroName() = "LLONG_MAX" or
24-
mie.getMacroName() = "INT_MAX" or
25-
mie.getMacroName() = "SHRT_MAX" or
26-
mie.getMacroName() = "UINT_MAX"
21+
predicate isMaxValue(Expr mie) {
22+
exists(MacroInvocation mi |
23+
mi.getExpr() = mie and
24+
(
25+
mi.getMacroName() = "CHAR_MAX" or
26+
mi.getMacroName() = "LLONG_MAX" or
27+
mi.getMacroName() = "INT_MAX" or
28+
mi.getMacroName() = "SHRT_MAX" or
29+
mi.getMacroName() = "UINT_MAX"
30+
)
31+
)
2732
}
2833

29-
predicate isMinValue(MacroInvocationExpr mie) {
30-
mie.getMacroName() = "CHAR_MIN" or
31-
mie.getMacroName() = "LLONG_MIN" or
32-
mie.getMacroName() = "INT_MIN" or
33-
mie.getMacroName() = "SHRT_MIN"
34+
predicate isMinValue(Expr mie) {
35+
exists(MacroInvocation mi |
36+
mi.getExpr() = mie and
37+
(
38+
mi.getMacroName() = "CHAR_MIN" or
39+
mi.getMacroName() = "LLONG_MIN" or
40+
mi.getMacroName() = "INT_MIN" or
41+
mi.getMacroName() = "SHRT_MIN"
42+
)
43+
)
3444
}
3545

3646
class SecurityOptionsArith extends SecurityOptions {

‎cpp/ql/src/semmle/code/cpp/Macro.qll‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,13 @@ class MacroInvocation extends MacroAccess {
261261

262262
/**
263263
* A top-level expression generated by a macro invocation.
264+
*
265+
* DEPRECATED: Use `MacroInvocation.getExpr()` directly to get an
266+
* expression generated at the top-level of a macro invocation. Use
267+
* `MacroInvocation.getAnAffectedElement()` to get any element generated
268+
* by a macro invocation.
264269
*/
265-
class MacroInvocationExpr extends Expr {
270+
deprecated class MacroInvocationExpr extends Expr {
266271
MacroInvocationExpr() {
267272
exists(MacroInvocation i | this = i.getExpr())
268273
}
@@ -282,8 +287,13 @@ class MacroInvocationExpr extends Expr {
282287

283288
/**
284289
* A top-level statement generated by a macro invocation.
290+
*
291+
* DEPRECATED: Use `MacroInvocation.getStmt()` directly to get a
292+
* statement generated at the top-level of a macro invocation. Use
293+
* `MacroInvocation.getAnAffectedElement()` to get any element generated
294+
* by a macro invocation.
285295
*/
286-
class MacroInvocationStmt extends Stmt {
296+
deprecated class MacroInvocationStmt extends Stmt {
287297
MacroInvocationStmt() {
288298
exists(MacroInvocation i | this = i.getStmt())
289299
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
1-
| test.c:13:17:13:17 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:10:13:10:16 | call to rand | Uncontrolled value |
1+
| 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 |
2+
| 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 |
3+
| test.c:40:5:40:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:39:13:39:21 | ... % ... | Uncontrolled value |
4+
| test.c:40:5:40:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:39:13:39:22 | call to rand | Uncontrolled value |
5+
| 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 |
6+
| test.c:56:5:56:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:54:13:54:16 | call to rand | Uncontrolled value |
7+
| test.c:67:5:67:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:66:13:66:16 | call to rand | Uncontrolled value |
8+
| 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 |
9+
| 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 |
10+
| 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 |
11+
| 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 |

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

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
// Semmle test case for rule ArithmeticUncontrolled.ql (Uncontrolled data in arithmetic expression).
22
// Associated with CWE-190: Integer Overflow or Wraparound. http://cwe.mitre.org/data/definitions/190.html
33

4-
int rand();
4+
int rand(void);
55
void trySlice(int start, int end);
66

7+
#define RAND() rand()
8+
#define RANDN(n) (rand() % n)
9+
#define RAND2() (rand() ^ rand())
10+
11+
12+
13+
14+
715
void randomTester() {
816
int i;
917
for (i = 0; i < 1000; i++) {
@@ -21,5 +29,74 @@ void randomTester() {
2129
trySlice(r, r+100);
2230
}
2331
}
24-
}
2532

33+
{
34+
int r = RAND();
35+
r += 100; // BAD: The return from RAND() is unbounded
36+
}
37+
38+
{
39+
int r = RANDN(100);
40+
r += 100; // GOOD: The return from RANDN is bounded [FALSE POSITIVE]
41+
}
42+
43+
{
44+
int r = rand();
45+
r += 100; // BAD
46+
}
47+
48+
{
49+
int r = rand() / 10;
50+
r += 100; // GOOD
51+
}
52+
53+
{
54+
int r = rand();
55+
r = r / 10;
56+
r += 100; // GOOD [FALSE POSITIVE]
57+
}
58+
59+
{
60+
int r = rand();
61+
r /= 10;
62+
r += 100; // GOOD
63+
}
64+
65+
{
66+
int r = rand() & 0xFF;
67+
r += 100; // GOOD [FALSE POSITIVE]
68+
}
69+
70+
{
71+
int r = rand() + 100; // BAD [NOT DETECTED]
72+
}
73+
74+
{
75+
int r = RAND2();
76+
77+
r = r - 100; // BAD
78+
}
79+
80+
{
81+
int r = (rand() ^ rand());
82+
83+
r = r - 100; // BAD [NOT DETECTED]
84+
}
85+
86+
{
87+
int r = RAND2() - 100; // BAD [NOT DETECTED]
88+
}
89+
90+
{
91+
int r = RAND();
92+
int *ptr_r = &r;
93+
*ptr_r -= 100; // BAD [NOT DETECTED]
94+
}
95+
96+
{
97+
int r = 0;
98+
int *ptr_r = &r;
99+
*ptr_r = RAND();
100+
r -= 100; // BAD
101+
}
102+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Semmle test case for rule ArithmeticUncontrolled.ql (Uncontrolled data in arithmetic expression).
2+
// Associated with CWE-190: Integer Overflow or Wraparound. http://cwe.mitre.org/data/definitions/190.html
3+
4+
int rand(void);
5+
6+
int get_rand()
7+
{
8+
return rand();
9+
}
10+
11+
void get_rand2(int *dest)
12+
{
13+
*dest = rand();
14+
}
15+
16+
void get_rand3(int &dest)
17+
{
18+
dest = rand();
19+
}
20+
21+
void randomTester2()
22+
{
23+
{
24+
int r = get_rand();
25+
r = r + 100; // BAD
26+
}
27+
28+
{
29+
int r;
30+
get_rand2(&r);
31+
r = r + 100; // BAD [NOT DETECTED]
32+
}
33+
34+
{
35+
int r;
36+
get_rand3(r);
37+
r = r + 100; // BAD
38+
}
39+
}

0 commit comments

Comments
 (0)