Skip to content

Commit c7d624d

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#8247 from ihsinme/ihsinme-patch-80
CPP: Add query for CWE-190: Integer Overflow or Wraparound when using transform after operation
2 parents e7dca43 + b32be69 commit c7d624d

File tree

6 files changed

+159
-0
lines changed

6 files changed

+159
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
...
2+
vUnsignedLong = (unsigned long)(vUnsignedInt*vUnsignedInt); // BAD
3+
...
4+
vUnsignedLong = ((unsigned long)vUnsignedInt*vUnsignedInt); // GOOD
5+
...

‎cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.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>Search for places where the result of the multiplication is subjected to explicit conversion, not the arguments. Therefore, during the multiplication period, you can lose meaningful data.</p>
7+
8+
9+
</overview>
10+
11+
<example>
12+
<p>The following example demonstrates erroneous and fixed methods for working with type conversion.</p>
13+
<sample src="DangerousUseOfTransformationAfterOperation.cpp" />
14+
15+
</example>
16+
<references>
17+
18+
<li>
19+
CERT C Coding Standard:
20+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap">INT30-C. Ensure that unsigned integer operations do not wrap</a>.
21+
</li>
22+
23+
</references>
24+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.ql

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/**
2+
* @name Dangerous use of transformation after operation.
3+
* @description By using the transformation after the operation, you are doing a pointless and dangerous action.
4+
* @kind problem
5+
* @id cpp/dangerous-use-of-transformation-after-operation
6+
* @problem.severity warning
7+
* @precision medium
8+
* @tags correctness
9+
* security
10+
* external/cwe/cwe-190
11+
*/
12+
13+
import cpp
14+
15+
/** Returns the number of the expression in the function call arguments. */
16+
int argumentPosition(FunctionCall fc, Expr exp, int n) {
17+
n in [0 .. fc.getNumberOfArguments() - 1] and
18+
fc.getArgument(n) = exp and
19+
result = n
20+
}
21+
22+
/** Holds if a nonsensical type conversion situation is found. */
23+
predicate conversionDoneLate(MulExpr mexp) {
24+
exists(Expr e1, Expr e2 |
25+
mexp.hasOperands(e1, e2) and
26+
not e1.isConstant() and
27+
not e1.hasConversion() and
28+
not e1.hasConversion() and
29+
(
30+
e2.isConstant() or
31+
not e2.hasConversion()
32+
) and
33+
mexp.getConversion().hasExplicitConversion() and
34+
mexp.getConversion() instanceof ParenthesisExpr and
35+
mexp.getConversion().getConversion() instanceof CStyleCast and
36+
mexp.getConversion().getConversion().getType().getSize() > mexp.getType().getSize() and
37+
mexp.getConversion().getConversion().getType().getSize() > e2.getType().getSize() and
38+
mexp.getConversion().getConversion().getType().getSize() > e1.getType().getSize() and
39+
exists(Expr e0 |
40+
e0.(AssignExpr).getRValue() = mexp.getParent*() and
41+
e0.(AssignExpr).getLValue().getType().getSize() =
42+
mexp.getConversion().getConversion().getType().getSize()
43+
or
44+
mexp.getEnclosingElement().(ComparisonOperation).hasOperands(mexp, e0) and
45+
e0.getType().getSize() = mexp.getConversion().getConversion().getType().getSize()
46+
or
47+
e0.(FunctionCall)
48+
.getTarget()
49+
.getParameter(argumentPosition(e0.(FunctionCall), mexp, _))
50+
.getType()
51+
.getSize() = mexp.getConversion().getConversion().getType().getSize()
52+
)
53+
)
54+
}
55+
56+
/** Holds if the situation of a possible signed overflow used in pointer arithmetic is found. */
57+
predicate signSmallerWithEqualSizes(MulExpr mexp) {
58+
exists(Expr e1, Expr e2 |
59+
mexp.hasOperands(e1, e2) and
60+
not e1.isConstant() and
61+
not e1.hasConversion() and
62+
not e1.hasConversion() and
63+
(
64+
e2.isConstant() or
65+
not e2.hasConversion()
66+
) and
67+
mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
68+
(
69+
e2.isConstant() or
70+
mexp.getConversion+().getUnderlyingType().getSize() = e2.getUnderlyingType().getSize()
71+
) and
72+
mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
73+
exists(AssignExpr ae |
74+
ae.getRValue() = mexp.getParent*() and
75+
ae.getRValue().getUnderlyingType().(IntegralType).isUnsigned() and
76+
ae.getLValue().getUnderlyingType().(IntegralType).isSigned() and
77+
(
78+
not exists(DivExpr de | mexp.getParent*() = de)
79+
or
80+
exists(DivExpr de, Expr ec |
81+
e2.isConstant() and
82+
de.hasOperands(mexp.getParent*(), ec) and
83+
ec.isConstant() and
84+
e2.getValue().toInt() > ec.getValue().toInt()
85+
)
86+
) and
87+
exists(PointerAddExpr pa |
88+
ae.getASuccessor+() = pa and
89+
pa.getAnOperand().(VariableAccess).getTarget() = ae.getLValue().(VariableAccess).getTarget()
90+
)
91+
)
92+
)
93+
}
94+
95+
from MulExpr mexp, string msg
96+
where
97+
conversionDoneLate(mexp) and
98+
msg = "This transformation is applied after multiplication."
99+
or
100+
signSmallerWithEqualSizes(mexp) and
101+
msg = "Possible signed overflow followed by offset of the pointer out of bounds."
102+
select mexp, msg

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| test.cpp:9:8:9:12 | ... * ... | Possible signed overflow followed by offset of the pointer out of bounds. |
2+
| test.cpp:13:24:13:28 | ... * ... | This transformation is applied after multiplication. |
3+
| test.cpp:16:28:16:32 | ... * ... | This transformation is applied after multiplication. |
4+
| test.cpp:19:22:19:26 | ... * ... | This transformation is applied after multiplication. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.qlref

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

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/test.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
void testCall (unsigned long);
2+
void functionWork(char aA[10],unsigned int aUI) {
3+
4+
unsigned long aL;
5+
char *aP;
6+
int aI;
7+
8+
aI = (aUI*8)/10; // GOOD
9+
aI = aUI*8; // BAD
10+
aP = aA+aI;
11+
aI = (int)aUI*8; // GOOD
12+
13+
aL = (unsigned long)(aI*aI); // BAD
14+
aL = ((unsigned long)aI*aI); // GOOD
15+
16+
testCall((unsigned long)(aI*aI)); // BAD
17+
testCall(((unsigned long)aI*aI)); // GOOD
18+
19+
if((unsigned long)(aI*aI) > aL) // BAD
20+
return;
21+
if(((unsigned long)aI*aI) > aL) // GOOD
22+
return;
23+
}

0 commit comments

Comments
 (0)