Skip to content

Commit 9504592

Browse files
MathiasVPMathiasVP
committed
C++: Promote cpp/incorrect-allocation-error-handling out of experimental.
1 parent c4f604b commit 9504592

File tree

9 files changed

+256
-256
lines changed

9 files changed

+256
-256
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.cpp renamed to ‎cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.cpp

File renamed without changes.

‎cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.qhelp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Different overloads of the <code>new</code> operator handle allocation failures in different ways.
7+
If <code>new T</code> fails for some type <code>T</code>, it throws a <code>std::bad_alloc</code> exception,
8+
but <code>new(std::nothrow) T</code> returns a null pointer. If the programmer does not use the corresponding
9+
method of error handling, allocation failure may go unhandled and could cause the program to behave in
10+
unexpected ways.</p>
11+
12+
</overview>
13+
<recommendation>
14+
15+
<p>Make sure that exceptions are handled appropriately if <code>new T</code> is used. On the other hand,
16+
make sure to handle the possibility of null pointers if <code>new(std::nothrow) T</code> is used.</p>
17+
18+
</recommendation>
19+
<example>
20+
<sample src="IncorrectAllocationErrorHandling.cpp" />
21+
22+
</example>
23+
<references>
24+
25+
<li>
26+
CERT C++ Coding Standard:
27+
<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
28+
</li>
29+
30+
</references>
31+
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/**
2+
* @name Incorrect allocation-error handling
3+
* @description `operator new` throws an exception on allocation failures, while `operator new(std::nothrow)` returns a null pointer. Mixing up these two failure conditions can result in unexpected behavior.
4+
* @kind problem
5+
* @id cpp/incorrect-allocation-error-handling
6+
* @problem.severity warning
7+
* @precision medium
8+
* @tags correctness
9+
* security
10+
* external/cwe/cwe-570
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
15+
import semmle.code.cpp.controlflow.Guards
16+
17+
/**
18+
* A C++ `delete` or `delete[]` expression.
19+
*/
20+
class DeleteOrDeleteArrayExpr extends Expr {
21+
DeleteOrDeleteArrayExpr() { this instanceof DeleteExpr or this instanceof DeleteArrayExpr }
22+
23+
DeallocationFunction getDeallocator() {
24+
result = [this.(DeleteExpr).getDeallocator(), this.(DeleteArrayExpr).getDeallocator()]
25+
}
26+
27+
Destructor getDestructor() {
28+
result = [this.(DeleteExpr).getDestructor(), this.(DeleteArrayExpr).getDestructor()]
29+
}
30+
}
31+
32+
/** Gets the `Constructor` invoked when `newExpr` allocates memory. */
33+
Constructor getConstructorForAllocation(NewOrNewArrayExpr newExpr) {
34+
result.getACallToThisFunction() = newExpr.getInitializer()
35+
}
36+
37+
/** Gets the `Destructor` invoked when `deleteExpr` deallocates memory. */
38+
Destructor getDestructorForDeallocation(DeleteOrDeleteArrayExpr deleteExpr) {
39+
result = deleteExpr.getDestructor()
40+
}
41+
42+
/** Holds if the evaluation of `newExpr` may throw an exception. */
43+
predicate newMayThrow(NewOrNewArrayExpr newExpr) {
44+
functionMayThrow(newExpr.getAllocator()) or
45+
functionMayThrow(getConstructorForAllocation(newExpr))
46+
}
47+
48+
/** Holds if the evaluation of `deleteExpr` may throw an exception. */
49+
predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) {
50+
functionMayThrow(deleteExpr.getDeallocator()) or
51+
functionMayThrow(getDestructorForDeallocation(deleteExpr))
52+
}
53+
54+
/**
55+
* Holds if the function may throw an exception when called. That is, if the body of the function looks
56+
* like it might throw an exception, and the function does not have a `noexcept` or `throw()` specifier.
57+
*/
58+
predicate functionMayThrow(Function f) {
59+
(not exists(f.getBlock()) or stmtMayThrow(f.getBlock())) and
60+
not f.isNoExcept() and
61+
not f.isNoThrow()
62+
}
63+
64+
/** Holds if the evaluation of `stmt` may throw an exception. */
65+
predicate stmtMayThrow(Stmt stmt) {
66+
stmtMayThrow(stmt.(BlockStmt).getAStmt())
67+
or
68+
convertedExprMayThrow(stmt.(ExprStmt).getExpr())
69+
or
70+
convertedExprMayThrow(stmt.(DeclStmt).getADeclaration().(Variable).getInitializer().getExpr())
71+
or
72+
exists(IfStmt ifStmt | ifStmt = stmt |
73+
convertedExprMayThrow(ifStmt.getCondition()) or
74+
stmtMayThrow([ifStmt.getThen(), ifStmt.getElse()])
75+
)
76+
or
77+
exists(ConstexprIfStmt constIfStmt | constIfStmt = stmt |
78+
stmtMayThrow([constIfStmt.getThen(), constIfStmt.getElse()])
79+
)
80+
or
81+
exists(Loop loop | loop = stmt |
82+
convertedExprMayThrow(loop.getCondition()) or
83+
stmtMayThrow(loop.getStmt())
84+
)
85+
or
86+
// The case for `Loop` already checked the condition and the statement.
87+
convertedExprMayThrow(stmt.(RangeBasedForStmt).getUpdate())
88+
or
89+
// The case for `Loop` already checked the condition and the statement.
90+
exists(ForStmt forStmt | forStmt = stmt |
91+
stmtMayThrow(forStmt.getInitialization())
92+
or
93+
convertedExprMayThrow(forStmt.getUpdate())
94+
)
95+
or
96+
exists(SwitchStmt switchStmt | switchStmt = stmt |
97+
convertedExprMayThrow(switchStmt.getExpr()) or
98+
stmtMayThrow(switchStmt.getStmt())
99+
)
100+
or
101+
// NOTE: We don't include `TryStmt` as those exceptions are not "observable" outside the function.
102+
stmtMayThrow(stmt.(Handler).getBlock())
103+
or
104+
convertedExprMayThrow(stmt.(CoReturnStmt).getExpr())
105+
or
106+
convertedExprMayThrow(stmt.(ReturnStmt).getExpr())
107+
}
108+
109+
/** Holds if the evaluation of `e` (including conversions) may throw an exception. */
110+
predicate convertedExprMayThrow(Expr e) {
111+
exprMayThrow(e)
112+
or
113+
convertedExprMayThrow(e.getConversion())
114+
}
115+
116+
/** Holds if the evaluation of `e` may throw an exception. */
117+
predicate exprMayThrow(Expr e) {
118+
e instanceof DynamicCast
119+
or
120+
e instanceof TypeidOperator
121+
or
122+
e instanceof ThrowExpr
123+
or
124+
newMayThrow(e)
125+
or
126+
deleteMayThrow(e)
127+
or
128+
convertedExprMayThrow(e.(UnaryOperation).getOperand())
129+
or
130+
exists(BinaryOperation binOp | binOp = e |
131+
convertedExprMayThrow([binOp.getLeftOperand(), binOp.getRightOperand()])
132+
)
133+
or
134+
exists(Assignment assign | assign = e |
135+
convertedExprMayThrow([assign.getLValue(), assign.getRValue()])
136+
)
137+
or
138+
exists(CommaExpr comma | comma = e |
139+
convertedExprMayThrow([comma.getLeftOperand(), comma.getRightOperand()])
140+
)
141+
or
142+
exists(StmtExpr stmtExpr | stmtExpr = e |
143+
convertedExprMayThrow(stmtExpr.getResultExpr()) or
144+
stmtMayThrow(stmtExpr.getStmt())
145+
)
146+
or
147+
convertedExprMayThrow(e.(Conversion).getExpr())
148+
or
149+
exists(FunctionCall fc | fc = e |
150+
not exists(fc.getTarget()) or
151+
functionMayThrow(fc.getTarget()) or
152+
convertedExprMayThrow(fc.getAnArgument())
153+
)
154+
}
155+
156+
/** An allocator that might throw an exception. */
157+
class ThrowingAllocator extends Function {
158+
ThrowingAllocator() {
159+
exists(NewOrNewArrayExpr newExpr |
160+
newExpr.getAllocator() = this and
161+
functionMayThrow(this)
162+
)
163+
}
164+
}
165+
166+
/** The `std::bad_alloc` exception and its `bsl` variant. */
167+
class BadAllocType extends Class {
168+
BadAllocType() { this.hasGlobalOrStdOrBslName("bad_alloc") }
169+
}
170+
171+
/**
172+
* A catch block that catches a `std::bad_alloc` (or any of its superclasses), or a catch
173+
* block that catches every exception (i.e., `catch(...)`).
174+
*/
175+
class BadAllocCatchBlock extends CatchBlock {
176+
BadAllocCatchBlock() {
177+
this.getParameter().getUnspecifiedType().stripType() =
178+
any(BadAllocType badAlloc).getABaseClass*()
179+
or
180+
not exists(this.getParameter())
181+
}
182+
}
183+
184+
/**
185+
* Holds if `newExpr` is embedded in a `try` statement with a catch block `catchBlock` that
186+
* catches a `std::bad_alloc` exception, but nothing in the `try` block (including the `newExpr`)
187+
* will throw that exception.
188+
*/
189+
predicate noThrowInTryBlock(NewOrNewArrayExpr newExpr, BadAllocCatchBlock catchBlock) {
190+
exists(TryStmt try |
191+
not stmtMayThrow(try.getStmt()) and
192+
try.getACatchClause() = catchBlock and
193+
newExpr.getEnclosingBlock().getEnclosingBlock*() = try.getStmt()
194+
)
195+
}
196+
197+
/**
198+
* Holds if `newExpr` is handles allocation failures by throwing an exception, yet
199+
* the guard condition `guard` compares the result of `newExpr` to a null value.
200+
*/
201+
predicate nullCheckInThrowingNew(NewOrNewArrayExpr newExpr, GuardCondition guard) {
202+
newExpr.getAllocator() instanceof ThrowingAllocator and
203+
(
204+
// Handles null comparisons.
205+
guard.ensuresEq(globalValueNumber(newExpr).getAnExpr(), any(NullValue null), _, _, _)
206+
or
207+
// Handles `if(ptr)` and `if(!ptr)` cases.
208+
guard = globalValueNumber(newExpr).getAnExpr()
209+
)
210+
}
211+
212+
from NewOrNewArrayExpr newExpr, Element element, string msg, string elementString
213+
where
214+
not newExpr.isFromUninstantiatedTemplate(_) and
215+
(
216+
noThrowInTryBlock(newExpr, element) and
217+
msg = "This allocation cannot throw. $@ is unnecessary." and
218+
elementString = "This catch block"
219+
or
220+
nullCheckInThrowingNew(newExpr, element) and
221+
msg = "This allocation cannot return null. $@ is unnecessary." and
222+
elementString = "This check"
223+
)
224+
select newExpr, msg, element, elementString

‎cpp/ql/src/experimental/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.qhelp

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)