Skip to content

Commit 7b596f4

Browse files
geoffw0geoffw0
authored
Merge pull request github#10431 from ihsinme/ihsinme-patch-111
CPP: Add query for CWE-369: Divide By Zero.
2 parents 1a19909 + 213abc6 commit 7b596f4

File tree

6 files changed

+610
-0
lines changed

6 files changed

+610
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
...
3+
a = getc(f);
4+
if (a < 123) ret = 123/a; // BAD
5+
...
6+
if (a != 0) ret = 123/a; // GOOD
7+
...

‎cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.qhelp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p> Possible cases of division by zero when using the return value from functions.</p>
7+
8+
</overview>
9+
10+
<example>
11+
<p>The following example shows the use of a function with an error when using the return value and without an error.</p>
12+
<sample src="DivideByZeroUsingReturnValue.cpp" />
13+
14+
</example>
15+
<references>
16+
17+
<li>
18+
CERT Coding Standard:
19+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT33-C.+Ensure+that+division+and+remainder+operations+do+not+result+in+divide-by-zero+errors">INT33-C. Ensure that division and remainder operations do not result in divide-by-zero errors - SEI CERT C Coding Standard - Confluence</a>.
20+
</li>
21+
22+
</references>
23+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.ql

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
/**
2+
* @name Divide by zero using return value
3+
* @description Possible cases of division by zero when using the return value from functions.
4+
* @kind problem
5+
* @id cpp/divide-by-zero-using-return-value
6+
* @problem.severity warning
7+
* @precision medium
8+
* @tags correctness
9+
* security
10+
* external/cwe/cwe-369
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
15+
import semmle.code.cpp.controlflow.Guards
16+
17+
/** Holds if function `fn` can return a value equal to value `val` */
18+
predicate mayBeReturnValue(Function fn, float val) {
19+
exists(Expr tmpExp, ReturnStmt rs |
20+
tmpExp.getValue().toFloat() = val and
21+
rs.getEnclosingFunction() = fn and
22+
(
23+
globalValueNumber(rs.getExpr()) = globalValueNumber(tmpExp)
24+
or
25+
exists(AssignExpr ae |
26+
ae.getLValue().(VariableAccess).getTarget() =
27+
globalValueNumber(rs.getExpr()).getAnExpr().(VariableAccess).getTarget() and
28+
globalValueNumber(ae.getRValue()) = globalValueNumber(tmpExp)
29+
)
30+
or
31+
exists(Initializer it |
32+
globalValueNumber(it.getExpr()) = globalValueNumber(tmpExp) and
33+
it.getDeclaration().(Variable).getAnAccess().getTarget() =
34+
globalValueNumber(rs.getExpr()).getAnExpr().(VariableAccess).getTarget()
35+
)
36+
)
37+
)
38+
}
39+
40+
/** Holds if function `fn` can return a value equal zero */
41+
predicate mayBeReturnZero(Function fn) {
42+
mayBeReturnValue(fn, 0)
43+
or
44+
fn.hasName([
45+
"iswalpha", "iswlower", "iswprint", "iswspace", "iswblank", "iswupper", "iswcntrl",
46+
"iswctype", "iswalnum", "iswgraph", "iswxdigit", "iswdigit", "iswpunct", "isblank", "isupper",
47+
"isgraph", "isalnum", "ispunct", "islower", "isspace", "isprint", "isxdigit", "iscntrl",
48+
"isdigit", "isalpha", "timespec_get", "feof", "atomic_is_lock_free",
49+
"atomic_compare_exchange", "thrd_equal", "isfinite", "islessequal", "isnan", "isgreater",
50+
"signbit", "isinf", "islessgreater", "isnormal", "isless", "isgreaterequal", "isunordered",
51+
"ferror"
52+
])
53+
or
54+
fn.hasName([
55+
"thrd_sleep", "feenv", "feholdexcept", "feclearexcept", "feexceptflag", "feupdateenv",
56+
"remove", "fflush", "setvbuf", "fgetpos", "fsetpos", "fclose", "rename", "fseek", "raise"
57+
])
58+
or
59+
fn.hasName(["tss_get", "gets"])
60+
or
61+
fn.hasName(["getc", "atoi"])
62+
}
63+
64+
/** Gets the Guard which compares the expression `bound` */
65+
pragma[inline]
66+
GuardCondition checkByValue(Expr bound, Expr val) {
67+
exists(GuardCondition gc |
68+
(
69+
gc.ensuresEq(bound, val, _, _, _) or
70+
gc.ensuresEq(val, bound, _, _, _) or
71+
gc.ensuresLt(bound, val, _, _, _) or
72+
gc.ensuresLt(val, bound, _, _, _) or
73+
gc = globalValueNumber(bound).getAnExpr()
74+
) and
75+
result = gc
76+
)
77+
}
78+
79+
/** Holds if there are no comparisons between the value returned by possible function calls `compArg` and the value `valArg`, or when these comparisons do not exclude equality to the value `valArg`. */
80+
pragma[inline]
81+
predicate compareFunctionWithValue(Expr guardExp, Function compArg, Expr valArg) {
82+
not exists(Expr exp |
83+
exp.getAChild*() = globalValueNumber(compArg.getACallToThisFunction()).getAnExpr() and
84+
checkByValue(exp, valArg).controls(guardExp.getBasicBlock(), _)
85+
)
86+
or
87+
exists(GuardCondition gc |
88+
(
89+
gc.ensuresEq(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), valArg, 0,
90+
guardExp.getBasicBlock(), true)
91+
or
92+
gc.ensuresEq(valArg, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0,
93+
guardExp.getBasicBlock(), true)
94+
or
95+
gc.ensuresLt(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), valArg, 0,
96+
guardExp.getBasicBlock(), false)
97+
or
98+
gc.ensuresLt(valArg, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0,
99+
guardExp.getBasicBlock(), false)
100+
)
101+
or
102+
exists(Expr exp |
103+
exp.getValue().toFloat() > valArg.getValue().toFloat() and
104+
gc.ensuresLt(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), exp, 0,
105+
guardExp.getBasicBlock(), true)
106+
or
107+
exp.getValue().toFloat() < valArg.getValue().toFloat() and
108+
gc.ensuresLt(exp, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0,
109+
guardExp.getBasicBlock(), true)
110+
)
111+
)
112+
or
113+
valArg.getValue().toFloat() = 0 and
114+
exists(NotExpr ne, IfStmt ifne |
115+
ne.getOperand() = globalValueNumber(compArg.getACallToThisFunction()).getAnExpr() and
116+
ifne.getCondition() = ne and
117+
ifne.getThen().getAChild*() = guardExp
118+
)
119+
}
120+
121+
/** Wraping predicate for call `compareFunctionWithValue`. */
122+
pragma[inline]
123+
predicate checkConditions1(Expr div, Function fn, float changeInt) {
124+
exists(Expr val |
125+
val.getEnclosingFunction() = fn and
126+
val.getValue().toFloat() = changeInt and
127+
compareFunctionWithValue(div, fn, val)
128+
)
129+
}
130+
131+
/** Holds if there are no comparisons between the value `compArg` and the value `valArg`, or when these comparisons do not exclude equality to the value `valArg`. */
132+
pragma[inline]
133+
predicate compareExprWithValue(Expr guardExp, Expr compArg, Expr valArg) {
134+
not exists(Expr exp |
135+
exp.getAChild*() = globalValueNumber(compArg).getAnExpr() and
136+
checkByValue(exp, valArg).controls(guardExp.getBasicBlock(), _)
137+
)
138+
or
139+
exists(GuardCondition gc |
140+
(
141+
gc.ensuresEq(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(), true)
142+
or
143+
gc.ensuresEq(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true)
144+
or
145+
gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(),
146+
false)
147+
or
148+
gc.ensuresLt(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(),
149+
false)
150+
)
151+
or
152+
exists(Expr exp |
153+
exp.getValue().toFloat() > valArg.getValue().toFloat() and
154+
gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), exp, 0, guardExp.getBasicBlock(), true)
155+
or
156+
exp.getValue().toFloat() < valArg.getValue().toFloat() and
157+
gc.ensuresLt(exp, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true)
158+
)
159+
)
160+
or
161+
valArg.getValue().toFloat() = 0 and
162+
exists(NotExpr ne, IfStmt ifne |
163+
ne.getOperand() = globalValueNumber(compArg).getAnExpr() and
164+
ifne.getCondition() = ne and
165+
ifne.getThen().getAChild*() = guardExp
166+
)
167+
}
168+
169+
/** Wraping predicate for call `compareExprWithValue`. */
170+
pragma[inline]
171+
predicate checkConditions2(Expr div, Expr divVal, float changeInt2) {
172+
exists(Expr val |
173+
(
174+
val.getEnclosingFunction() =
175+
div.getEnclosingFunction().getACallToThisFunction().getEnclosingFunction() or
176+
val.getEnclosingFunction() = div.getEnclosingFunction()
177+
) and
178+
val.getValue().toFloat() = changeInt2 and
179+
compareExprWithValue(div, divVal, val)
180+
)
181+
}
182+
183+
/** Gets the value of the difference or summand from the expression `src`. */
184+
float getValueOperand(Expr src, Expr e1, Expr e2) {
185+
src.(SubExpr).hasOperands(e1, e2) and
186+
result = e2.getValue().toFloat()
187+
or
188+
src.(AddExpr).hasOperands(e1, e2) and
189+
result = -e2.getValue().toFloat()
190+
}
191+
192+
/** Function the return of the expression `e1` and the multiplication operands, or the left operand of division if `e1` contains a multiplication or division, respectively. */
193+
Expr getMulDivOperand(Expr e1) {
194+
result = e1 or
195+
result = e1.(MulExpr).getAnOperand() or
196+
result = e1.(DivExpr).getLeftOperand()
197+
}
198+
199+
/** The class that defines possible variants of the division expression or the search for the remainder. */
200+
class MyDiv extends Expr {
201+
MyDiv() {
202+
this instanceof DivExpr or
203+
this instanceof RemExpr or
204+
this instanceof AssignDivExpr or
205+
this instanceof AssignRemExpr
206+
}
207+
208+
Expr getRV() {
209+
result = this.(AssignArithmeticOperation).getRValue() or
210+
result = this.(BinaryArithmeticOperation).getRightOperand()
211+
}
212+
}
213+
214+
from Expr exp, string msg, Function fn, GVN findVal, float changeInt, MyDiv div
215+
where
216+
findVal = globalValueNumber(fn.getACallToThisFunction()) and
217+
(
218+
// Look for divide-by-zero operations possible due to the return value of the function `fn`.
219+
checkConditions1(div, fn, changeInt) and
220+
(
221+
// Function return value can be zero.
222+
mayBeReturnZero(fn) and
223+
getMulDivOperand(globalValueNumber(div.getRV()).getAnExpr()) = findVal.getAnExpr() and
224+
changeInt = 0
225+
or
226+
// Denominator can be sum or difference.
227+
changeInt = getValueOperand(div.getRV(), findVal.getAnExpr(), _) and
228+
mayBeReturnValue(fn, changeInt)
229+
) and
230+
exp = div and
231+
msg =
232+
"Can lead to division by 0, since the function " + fn.getName() + " can return a value " +
233+
changeInt.toString() + "."
234+
or
235+
// Search for situations where division by zero is possible inside the `divFn` function if the passed argument can be equal to a certain value.
236+
exists(int posArg, Expr divVal, FunctionCall divFc, float changeInt2 |
237+
// Division is associated with the function argument.
238+
exists(Function divFn |
239+
divFn.getParameter(posArg).getAnAccess() = divVal and
240+
divVal.getEnclosingStmt() = div.getEnclosingStmt() and
241+
divFc = divFn.getACallToThisFunction()
242+
) and
243+
(
244+
divVal = div.getRV() and
245+
divFc.getArgument(posArg) != findVal.getAnExpr() and
246+
(
247+
// Function return value can be zero.
248+
mayBeReturnZero(fn) and
249+
getMulDivOperand(globalValueNumber(divFc.getArgument(posArg)).getAnExpr()) =
250+
findVal.getAnExpr() and
251+
changeInt = 0 and
252+
changeInt2 = 0
253+
or
254+
// Denominator can be sum or difference.
255+
changeInt = getValueOperand(divFc.getArgument(posArg), findVal.getAnExpr(), _) and
256+
mayBeReturnValue(fn, changeInt) and
257+
changeInt2 = 0
258+
)
259+
or
260+
// Look for a situation where the difference or subtraction is considered as an argument, and it can be used in the same way.
261+
changeInt = getValueOperand(div.getRV(), divVal, _) and
262+
changeInt2 = changeInt and
263+
mayBeReturnValue(fn, changeInt) and
264+
divFc.getArgument(posArg) = findVal.getAnExpr()
265+
) and
266+
checkConditions2(div, divVal, changeInt2) and
267+
checkConditions1(divFc, fn, changeInt) and
268+
exp = divFc and
269+
msg =
270+
"Can lead to division by 0, since the function " + fn.getName() + " can return a value " +
271+
changeInt.toString() + "."
272+
)
273+
)
274+
select exp, msg

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.expected

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
| test.cpp:47:24:47:31 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
2+
| test.cpp:48:15:48:34 | ... / ... | Can lead to division by 0, since the function getSize2 can return a value 0. |
3+
| test.cpp:53:10:53:17 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
4+
| test.cpp:65:15:65:22 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
5+
| test.cpp:68:15:68:22 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
6+
| test.cpp:71:9:71:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
7+
| test.cpp:74:9:74:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
8+
| test.cpp:77:21:77:28 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
9+
| test.cpp:79:25:79:32 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
10+
| test.cpp:81:24:81:31 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
11+
| test.cpp:128:10:128:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
12+
| test.cpp:135:10:135:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
13+
| test.cpp:141:10:141:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
14+
| test.cpp:153:12:153:19 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. |
15+
| test.cpp:172:3:172:12 | ... /= ... | Can lead to division by 0, since the function getSize can return a value 0. |
16+
| test.cpp:173:3:173:12 | ... %= ... | Can lead to division by 0, since the function getSize can return a value 0. |
17+
| test.cpp:187:10:187:17 | ... / ... | Can lead to division by 0, since the function getSizeFloat can return a value 0. |
18+
| test.cpp:199:12:199:25 | ... / ... | Can lead to division by 0, since the function getSize can return a value -1. |
19+
| test.cpp:202:12:202:25 | ... / ... | Can lead to division by 0, since the function getSize can return a value 1. |
20+
| test.cpp:205:10:205:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 1. |
21+
| test.cpp:210:10:210:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 3. |
22+
| test.cpp:258:3:258:10 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 0. |
23+
| test.cpp:259:3:259:10 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 2. |
24+
| test.cpp:260:3:260:13 | call to badMySubDiv | Can lead to division by 0, since the function getSize can return a value 3. |
25+
| test.cpp:263:5:263:15 | call to badMySubDiv | Can lead to division by 0, since the function getSize can return a value 3. |
26+
| test.cpp:273:5:273:12 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 3. |
27+
| test.cpp:275:5:275:12 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value -1. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.qlref

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

0 commit comments

Comments
 (0)