Skip to content

Commit 43044cd

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#6081 from ihsinme/ihsinme-patch-273
CPP: Add a query to find incorrectly used switch
2 parents ad86641 + 098773d commit 43044cd

File tree

6 files changed

+276
-0
lines changed

6 files changed

+276
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
...
2+
int i1;
3+
char c1;
4+
...
5+
if((c1<50)&&(c>10))
6+
switch(c1){
7+
case 300: // BAD: the code will not be executed
8+
...
9+
if((i1<5)&&(i1>0))
10+
switch(i1){ // BAD
11+
case 21: // BAD: the code will not be executed
12+
...
13+
switch(c1){
14+
...
15+
dafault: // BAD: maybe it will be right `default`
16+
...
17+
}
18+
19+
...
20+
switch(c1){
21+
i1=c1*2; // BAD: the code will not be executed
22+
case 12:
23+
...
24+
switch(c1){ // GOOD
25+
case 12:
26+
break;
27+
case 10:
28+
break;
29+
case 9:
30+
break;
31+
default:
32+
break;
33+
}
34+
...

‎cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.qhelp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>A mismatch between conditionals and <code>switch</code> cases can lead to control-flow violations (CWE-691) where the developer either does not handle all combinations of conditions or unintentionally created dead code (CWE-561).</p>
7+
8+
9+
</overview>
10+
11+
<example>
12+
<p>The following example demonstrates fallacious and fixed ways of using a <code>switch</code> statement.</p>
13+
<sample src="FindIncorrectlyUsedSwitch.c" />
14+
15+
</example>
16+
<references>
17+
18+
<li>
19+
CERT C Coding Standard:
20+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.
21+
</li>
22+
23+
</references>
24+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.ql

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/**
2+
* @name Incorrect switch statement
3+
* @description --Finding places the dangerous use of a switch.
4+
* --For example, when the range of values for a condition does not cover all of the selection values..
5+
* @kind problem
6+
* @id cpp/operator-find-incorrectly-used-switch
7+
* @problem.severity warning
8+
* @precision medium
9+
* @tags correctness
10+
* security
11+
* external/cwe/cwe-561
12+
* external/cwe/cwe-691
13+
* external/cwe/cwe-478
14+
*/
15+
16+
import cpp
17+
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
18+
19+
/** Holds if the range contains no boundary values. */
20+
predicate isRealRange(Expr exp) {
21+
upperBound(exp).toString() != "18446744073709551616" and
22+
upperBound(exp).toString() != "9223372036854775807" and
23+
upperBound(exp).toString() != "4294967295" and
24+
upperBound(exp).toString() != "Infinity" and
25+
upperBound(exp).toString() != "NaN" and
26+
lowerBound(exp).toString() != "-9223372036854775808" and
27+
lowerBound(exp).toString() != "-4294967296" and
28+
lowerBound(exp).toString() != "-Infinity" and
29+
lowerBound(exp).toString() != "NaN" and
30+
upperBound(exp) != 2147483647 and
31+
upperBound(exp) != 268435455 and
32+
upperBound(exp) != 33554431 and
33+
upperBound(exp) != 8388607 and
34+
upperBound(exp) != 65535 and
35+
upperBound(exp) != 32767 and
36+
upperBound(exp) != 255 and
37+
upperBound(exp) != 127 and
38+
upperBound(exp) != 63 and
39+
upperBound(exp) != 31 and
40+
upperBound(exp) != 15 and
41+
upperBound(exp) != 7 and
42+
lowerBound(exp) != -2147483648 and
43+
lowerBound(exp) != -268435456 and
44+
lowerBound(exp) != -33554432 and
45+
lowerBound(exp) != -8388608 and
46+
lowerBound(exp) != -65536 and
47+
lowerBound(exp) != -32768 and
48+
lowerBound(exp) != -128
49+
}
50+
51+
/** Holds if the range of values for the condition is less than the choices. */
52+
predicate isNotAllSelected(SwitchStmt swtmp) {
53+
not swtmp.getExpr().isConstant() and
54+
exists(int i |
55+
i != 0 and
56+
(
57+
i = lowerBound(swtmp.getASwitchCase().getExpr()) and
58+
upperBound(swtmp.getExpr()) < i
59+
or
60+
(
61+
i = upperBound(swtmp.getASwitchCase().getExpr()) or
62+
i = upperBound(swtmp.getASwitchCase().getEndExpr())
63+
) and
64+
lowerBound(swtmp.getExpr()) > i
65+
)
66+
)
67+
}
68+
69+
/** Holds if the range of values for the condition is greater than the selection. */
70+
predicate isConditionBig(SwitchStmt swtmp) {
71+
not swtmp.hasDefaultCase() and
72+
not exists(int iu, int il |
73+
(
74+
iu = upperBound(swtmp.getASwitchCase().getExpr()) or
75+
iu = upperBound(swtmp.getASwitchCase().getEndExpr())
76+
) and
77+
upperBound(swtmp.getExpr()) = iu and
78+
(
79+
il = lowerBound(swtmp.getASwitchCase().getExpr()) or
80+
il = lowerBound(swtmp.getASwitchCase().getEndExpr())
81+
) and
82+
lowerBound(swtmp.getExpr()) = il
83+
)
84+
}
85+
86+
/** Holds if there are labels inside the block with names similar to `default` or `case`. */
87+
predicate isWrongLableName(SwitchStmt swtmp) {
88+
not swtmp.hasDefaultCase() and
89+
exists(LabelStmt lb |
90+
(
91+
(
92+
lb.getName().charAt(0) = "d" or
93+
lb.getName().charAt(0) = "c"
94+
) and
95+
(
96+
lb.getName().charAt(1) = "e" or
97+
lb.getName().charAt(1) = "a"
98+
) and
99+
(
100+
lb.getName().charAt(2) = "f" or
101+
lb.getName().charAt(2) = "s"
102+
)
103+
) and
104+
lb.getEnclosingStmt().getParentStmt*() = swtmp.getStmt() and
105+
not exists(GotoStmt gs | gs.getName() = lb.getName())
106+
)
107+
}
108+
109+
/** Holds if the block contains code before the first `case`. */
110+
predicate isCodeBeforeCase(SwitchStmt swtmp) {
111+
exists(Expr exp |
112+
exp.getEnclosingStmt().getParentStmt*() = swtmp.getStmt() and
113+
not exists(Loop lp |
114+
exp.getEnclosingStmt().getParentStmt*() = lp and
115+
lp.getEnclosingStmt().getParentStmt*() = swtmp.getStmt()
116+
) and
117+
not exists(Stmt sttmp, SwitchCase sctmp |
118+
sttmp = swtmp.getASwitchCase().getAStmt() and
119+
sctmp = swtmp.getASwitchCase() and
120+
(
121+
exp.getEnclosingStmt().getParentStmt*() = sttmp or
122+
exp.getEnclosingStmt() = sctmp
123+
)
124+
)
125+
)
126+
}
127+
128+
from SwitchStmt sw, string msg
129+
where
130+
isRealRange(sw.getExpr()) and
131+
lowerBound(sw.getExpr()) != upperBound(sw.getExpr()) and
132+
lowerBound(sw.getExpr()) != 0 and
133+
not exists(Expr cexp |
134+
cexp = sw.getASwitchCase().getExpr() and not isRealRange(cexp)
135+
or
136+
cexp = sw.getASwitchCase().getEndExpr() and not isRealRange(cexp)
137+
) and
138+
not exists(Expr exptmp |
139+
exptmp = sw.getExpr().getAChild*() and
140+
not exptmp.isConstant() and
141+
not isRealRange(exptmp)
142+
) and
143+
(sw.getASwitchCase().terminatesInBreakStmt() or sw.getASwitchCase().terminatesInReturnStmt()) and
144+
(
145+
isNotAllSelected(sw) and msg = "The range of condition values is less than the selection."
146+
or
147+
isConditionBig(sw) and msg = "The range of condition values is wider than the choices."
148+
)
149+
or
150+
isWrongLableName(sw) and msg = "Possibly erroneous label name."
151+
or
152+
isCodeBeforeCase(sw) and msg = "Code before case will not be executed."
153+
select sw, msg

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/FindIncorrectlyUsedSwitch.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| test.c:20:3:28:3 | switch (...) ... | Possibly erroneous label name. |
2+
| test.c:30:3:38:3 | switch (...) ... | Code before case will not be executed. |
3+
| test.c:41:3:50:3 | switch (...) ... | The range of condition values is less than the selection. |
4+
| test.c:53:3:58:3 | switch (...) ... | The range of condition values is wider than the choices. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/FindIncorrectlyUsedSwitch.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/test.c

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
void testFunction(char c1,int i1)
2+
{
3+
4+
switch(c1){ // GOOD
5+
case 12:
6+
break;
7+
case 10:
8+
break;
9+
case 9:
10+
break;
11+
}
12+
13+
switch(i1){ // GOOD
14+
for(i1=0;i1<20;i1++){
15+
case 12:
16+
case 10:
17+
case 9:
18+
}
19+
}
20+
switch(c1){ // BAD
21+
case 12:
22+
break;
23+
case 10:
24+
break;
25+
case 9:
26+
break;
27+
dafault:
28+
}
29+
30+
switch(c1){ // BAD
31+
c1=c1*2;
32+
case 12:
33+
break;
34+
case 10:
35+
break;
36+
case 9:
37+
break;
38+
}
39+
40+
if((c1<6)&&(c1>0))
41+
switch(c1){ // BAD
42+
case 8:
43+
break;
44+
case 5:
45+
break;
46+
case 3:
47+
break;
48+
case 1:
49+
break;
50+
}
51+
52+
if((c1<6)&&(c1>0))
53+
switch(c1){ // BAD
54+
case 3:
55+
break;
56+
case 1:
57+
break;
58+
}
59+
60+
}

0 commit comments

Comments
 (0)