Skip to content

Commit 50105c1

Browse files
author
ihsinme
committed
create new branchihsinme-patch-111 in fork
1 parent 1478f61 commit 50105c1

File tree

6 files changed

+619
-0
lines changed

6 files changed

+619
-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: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
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, Expr val) {
19+
exists(Expr tmpExp, ReturnStmt rs |
20+
tmpExp.getValue().toFloat() = val.getValue().toFloat() 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().(VariableAccess).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+
exists(Expr zr | zr.getValue().toFloat() = 0 and mayBeReturnValue(fn, zr))
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+
/** The function returns 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+
or
101+
if valArg.getValue().toFloat() = 0
102+
then
103+
exists(NotExpr ne, IfStmt ifne |
104+
globalValueNumber(ne.getOperand()) = globalValueNumber(compArg.getACallToThisFunction()) and
105+
ifne.getCondition() = ne and
106+
ifne.getThen().getAChild*() = guardExp
107+
)
108+
else none()
109+
)
110+
or
111+
exists(Expr exp |
112+
exp.getValue().toFloat() > valArg.getValue().toFloat() and
113+
gc.ensuresLt(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), exp, 0,
114+
guardExp.getBasicBlock(), true)
115+
or
116+
exp.getValue().toFloat() < valArg.getValue().toFloat() and
117+
gc.ensuresLt(exp, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0,
118+
guardExp.getBasicBlock(), true)
119+
)
120+
)
121+
}
122+
123+
/** Wraping predicate for call `compareFunctionWithValue`. */
124+
pragma[inline]
125+
predicate checkConditions1(Expr div, Function fn, float changeInt) {
126+
exists(Expr val |
127+
val.getValue().toFloat() = changeInt and
128+
compareFunctionWithValue(div, fn, val)
129+
)
130+
}
131+
132+
/** 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`. */
133+
pragma[inline]
134+
predicate compareExprWithValue(Expr guardExp, Expr compArg, Expr valArg) {
135+
not exists(Expr exp |
136+
exp.getAChild*() = globalValueNumber(compArg).getAnExpr() and
137+
checkByValue(exp, valArg).controls(guardExp.getBasicBlock(), _)
138+
)
139+
or
140+
exists(GuardCondition gc |
141+
(
142+
gc.ensuresEq(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(), true)
143+
or
144+
gc.ensuresEq(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true)
145+
or
146+
gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(),
147+
false)
148+
or
149+
gc.ensuresLt(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(),
150+
false)
151+
or
152+
if valArg.getValue().toFloat() = 0
153+
then
154+
exists(NotExpr ne, IfStmt ifne |
155+
globalValueNumber(ne.getOperand()) = globalValueNumber(compArg) and
156+
ifne.getCondition() = ne and
157+
ifne.getThen().getAChild*() = guardExp
158+
)
159+
else none()
160+
)
161+
or
162+
exists(Expr exp |
163+
exp.getValue().toFloat() > valArg.getValue().toFloat() and
164+
gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), exp, 0, guardExp.getBasicBlock(), true)
165+
or
166+
exp.getValue().toFloat() < valArg.getValue().toFloat() and
167+
gc.ensuresLt(exp, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true)
168+
)
169+
)
170+
}
171+
172+
/** Wraping predicate for call `compareExprWithValue`. */
173+
pragma[inline]
174+
predicate checkConditions2(Expr div, Expr divVal, float changeInt2) {
175+
exists(Expr val |
176+
val.getValue().toFloat() = changeInt2 and
177+
compareExprWithValue(div, divVal, val)
178+
)
179+
}
180+
181+
/** The function returns the value of the difference or summand from the expression `src`. */
182+
float getValueOperand(Expr src, Expr e1, Expr e2) {
183+
src.(SubExpr).hasOperands(e1, e2) and
184+
result = e2.getValue().toFloat()
185+
or
186+
src.(AddExpr).hasOperands(e1, e2) and
187+
result = -e2.getValue().toFloat()
188+
}
189+
190+
/** 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. */
191+
Expr getMulDivOperand(Expr e1) {
192+
result = e1 or
193+
result = e1.(MulExpr).getAnOperand() or
194+
result = e1.(DivExpr).getLeftOperand()
195+
}
196+
197+
/** Class that defines possible variants of the division expression or the search for the remainder. */
198+
class MyDiv extends Expr {
199+
MyDiv() {
200+
this instanceof DivExpr or
201+
this instanceof RemExpr or
202+
this instanceof AssignDivExpr or
203+
this instanceof AssignRemExpr
204+
}
205+
206+
Expr getRV() {
207+
result = this.(AssignArithmeticOperation).getRValue() or
208+
result = this.(BinaryArithmeticOperation).getRightOperand()
209+
}
210+
}
211+
212+
from Expr exp, string msg, Function fn, Expr findVal, float changeInt, MyDiv div
213+
where
214+
findVal = globalValueNumber(fn.getACallToThisFunction()).getAnExpr() and
215+
(
216+
// Look for divide-by-zero operations possible due to the return value of the function `fn`.
217+
checkConditions1(div, fn, changeInt) and
218+
(
219+
// Function return value can be zero.
220+
mayBeReturnZero(fn) and
221+
getMulDivOperand(globalValueNumber(div.getRV()).getAnExpr()) = findVal and
222+
changeInt = 0
223+
or
224+
// Denominator can be sum or difference.
225+
exists(Expr changeExpr |
226+
mayBeReturnValue(fn, changeExpr) and
227+
changeInt = getValueOperand(div.getRV(), findVal, changeExpr)
228+
)
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+
divFc = divFn.getACallToThisFunction()
241+
) and
242+
(
243+
divVal = div.getRV() and
244+
(
245+
// Function return value can be zero.
246+
mayBeReturnZero(fn) and
247+
getMulDivOperand(globalValueNumber(divFc.getArgument(posArg)).getAnExpr()) = findVal and
248+
changeInt = 0 and
249+
changeInt2 = 0
250+
or
251+
// Denominator can be sum or difference.
252+
exists(Expr changeExpr |
253+
mayBeReturnValue(fn, changeExpr) and
254+
changeInt = getValueOperand(divFc.getArgument(posArg), findVal, changeExpr) and
255+
changeInt2 = 0
256+
)
257+
)
258+
or
259+
// Look for a situation where the difference or subtraction is considered as an argument, and it can be used in the same way.
260+
exists(Expr changeExpr |
261+
changeInt = getValueOperand(div.getRV(), divVal, changeExpr) and
262+
changeInt2 = changeInt and
263+
mayBeReturnValue(fn, changeExpr)
264+
) and
265+
divFc.getArgument(posArg) = findVal
266+
) and
267+
checkConditions2(div, divVal, changeInt2) and
268+
checkConditions1(divFc, fn, changeInt) and
269+
exp = divFc and
270+
msg =
271+
"Can lead to division by 0, since the function " + fn.getName() + " can return a value " +
272+
changeInt.toString() + "."
273+
)
274+
)
275+
select exp, msg

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

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