Skip to content

Commit e9bc5a5

Browse files
alexetalexet
committed
CPP: Add query for detecting invalid uses of temporary unique pointers.
1 parent 7006d00 commit e9bc5a5

File tree

9 files changed

+404
-75
lines changed

9 files changed

+404
-75
lines changed

‎cpp/ql/src/Security/CWE/CWE-416/Temporaries.qll

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import cpp
2+
import semmle.code.cpp.models.implementations.StdContainer
3+
4+
/**
5+
* Holds if `e` will be consumed by its parent as a glvalue and does not have
6+
* an lvalue-to-rvalue conversion. This means that it will be materialized into
7+
* a temporary object.
8+
*/
9+
predicate isTemporary(Expr e) {
10+
e instanceof TemporaryObjectExpr
11+
or
12+
e.isPRValueCategory() and
13+
e.getUnspecifiedType() instanceof Class and
14+
not e.hasLValueToRValueConversion()
15+
}
16+
17+
/** Holds if `e` is written to a container. */
18+
predicate isStoredInContainer(Expr e) {
19+
exists(StdSequenceContainerInsert insert, Call call, int index |
20+
call = insert.getACallToThisFunction() and
21+
index = insert.getAValueTypeParameterIndex() and
22+
call.getArgument(index) = e
23+
)
24+
or
25+
exists(StdSequenceContainerPush push, Call call, int index |
26+
call = push.getACallToThisFunction() and
27+
index = push.getAValueTypeParameterIndex() and
28+
call.getArgument(index) = e
29+
)
30+
or
31+
exists(StdSequenceEmplace emplace, Call call, int index |
32+
call = emplace.getACallToThisFunction() and
33+
index = emplace.getAValueTypeParameterIndex() and
34+
call.getArgument(index) = e
35+
)
36+
or
37+
exists(StdSequenceEmplaceBack emplaceBack, Call call, int index |
38+
call = emplaceBack.getACallToThisFunction() and
39+
index = emplaceBack.getAValueTypeParameterIndex() and
40+
call.getArgument(index) = e
41+
)
42+
}
43+
44+
/**
45+
* Holds if the value of `e` outlives the enclosing full expression. For
46+
* example, because the value is stored in a local variable.
47+
*/
48+
predicate outlivesFullExpr(Expr e) {
49+
not e.getConversion*().hasLValueToRValueConversion() and
50+
(
51+
any(Assignment assign).getRValue() = e
52+
or
53+
any(Variable v).getInitializer().getExpr() = e
54+
or
55+
any(ReturnStmt ret).getExpr() = e
56+
or
57+
exists(ConditionalExpr cond |
58+
outlivesFullExpr(cond) and
59+
[cond.getThen(), cond.getElse()] = e
60+
)
61+
or
62+
exists(BinaryOperation bin |
63+
outlivesFullExpr(bin) and
64+
bin.getAnOperand() = e and
65+
not bin instanceof ComparisonOperation
66+
)
67+
or
68+
exists(PointerFieldAccess fa |
69+
outlivesFullExpr(fa) and
70+
fa.getQualifier() = e
71+
)
72+
or
73+
exists(AddressOfExpr ao |
74+
outlivesFullExpr(ao) and
75+
ao.getOperand() = e
76+
)
77+
or
78+
exists(ClassAggregateLiteral aggr |
79+
outlivesFullExpr(aggr) and
80+
aggr.getAFieldExpr(_) = e
81+
)
82+
or
83+
exists(ArrayAggregateLiteral aggr |
84+
outlivesFullExpr(aggr) and
85+
aggr.getAnElementExpr(_) = e
86+
)
87+
or
88+
isStoredInContainer(e)
89+
)
90+
}

‎cpp/ql/src/Security/CWE/CWE-416/UseOfStringAfterLifetimeEnds.ql

Lines changed: 1 addition & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -14,81 +14,7 @@
1414

1515
import cpp
1616
import semmle.code.cpp.models.implementations.StdString
17-
import semmle.code.cpp.models.implementations.StdContainer
18-
19-
/**
20-
* Holds if `e` will be consumed by its parent as a glvalue and does not have
21-
* an lvalue-to-rvalue conversion. This means that it will be materialized into
22-
* a temporary object.
23-
*/
24-
predicate isTemporary(Expr e) {
25-
e instanceof TemporaryObjectExpr
26-
or
27-
e.isPRValueCategory() and
28-
e.getUnspecifiedType() instanceof Class and
29-
not e.hasLValueToRValueConversion()
30-
}
31-
32-
/** Holds if `e` is written to a container. */
33-
predicate isStoredInContainer(Expr e) {
34-
exists(StdSequenceContainerInsert insert, Call call, int index |
35-
call = insert.getACallToThisFunction() and
36-
index = insert.getAValueTypeParameterIndex() and
37-
call.getArgument(index) = e
38-
)
39-
or
40-
exists(StdSequenceContainerPush push, Call call, int index |
41-
call = push.getACallToThisFunction() and
42-
index = push.getAValueTypeParameterIndex() and
43-
call.getArgument(index) = e
44-
)
45-
or
46-
exists(StdSequenceEmplace emplace, Call call, int index |
47-
call = emplace.getACallToThisFunction() and
48-
index = emplace.getAValueTypeParameterIndex() and
49-
call.getArgument(index) = e
50-
)
51-
or
52-
exists(StdSequenceEmplaceBack emplaceBack, Call call, int index |
53-
call = emplaceBack.getACallToThisFunction() and
54-
index = emplaceBack.getAValueTypeParameterIndex() and
55-
call.getArgument(index) = e
56-
)
57-
}
58-
59-
/**
60-
* Holds if the value of `e` outlives the enclosing full expression. For
61-
* example, because the value is stored in a local variable.
62-
*/
63-
predicate outlivesFullExpr(Expr e) {
64-
any(Assignment assign).getRValue() = e
65-
or
66-
any(Variable v).getInitializer().getExpr() = e
67-
or
68-
any(ReturnStmt ret).getExpr() = e
69-
or
70-
exists(ConditionalExpr cond |
71-
outlivesFullExpr(cond) and
72-
[cond.getThen(), cond.getElse()] = e
73-
)
74-
or
75-
exists(BinaryOperation bin |
76-
outlivesFullExpr(bin) and
77-
bin.getAnOperand() = e
78-
)
79-
or
80-
exists(ClassAggregateLiteral aggr |
81-
outlivesFullExpr(aggr) and
82-
aggr.getAFieldExpr(_) = e
83-
)
84-
or
85-
exists(ArrayAggregateLiteral aggr |
86-
outlivesFullExpr(aggr) and
87-
aggr.getAnElementExpr(_) = e
88-
)
89-
or
90-
isStoredInContainer(e)
91-
}
17+
import Temporaries
9218

9319
from Call c
9420
where

‎cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.qhelp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Calling <code>get</code> on a <code>std::unique_ptr</code> object returns a pointer to the underlying allocations.
8+
When the <code>std::unique_ptr</code> object is destroyed, the pointer returned by <code>get</code> is no
9+
longer valid. If the pointer is used after the <code>std::unique_ptr</code> object is destroyed, then the behavior is undefined.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
Ensure that the pointer returned by <code>get</code> does not outlive the underlying <code>std::unique_ptr</code> object.
16+
</p>
17+
</recommendation>
18+
19+
<example>
20+
<p>
21+
The following example gets a <code>std::unique_ptr</code> object, and then converts the resulting unique pointer to a
22+
pointer using <code>get</code> so that it can be passed to the <code>work</code> function.
23+
24+
However, the <code>std::unique_ptr</code> object is destroyed as soon as the call
25+
to <code>get</code> returns. This means that <code>work</code> is given a pointer to invalid memory.
26+
</p>
27+
28+
<sample src="UseOfUniquePointerAfterLifetimeEndsBad.cpp" />
29+
30+
<p>
31+
The following example fixes the above code by ensuring that the pointer returned by the call to <code>get</code> does
32+
not outlive the underlying <code>std::unique_ptr</code> objects. This ensures that the pointer passed to <code>work</code>
33+
points to valid memory.
34+
</p>
35+
36+
<sample src="UseOfUniquePointerAfterLifetimeEndsGood.cpp" />
37+
38+
</example>
39+
<references>
40+
41+
<li><a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM50-CPP.+Do+not+access+freed+memory">MEM50-CPP. Do not access freed memory</a>.</li>
42+
43+
</references>
44+
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* @name Use of unique pointer after lifetime ends
3+
* @description If a reference to the contents of a unique pointer outlives the underlying object it may lead to unexpected behavior.
4+
* @kind problem
5+
* @precision high
6+
* @id cpp/use-of-uniwue-pointer-after-lifetime-ends
7+
* @problem.severity warning
8+
* @security-severity 8.8
9+
* @tags reliability
10+
* security
11+
* external/cwe/cwe-416
12+
* external/cwe/cwe-664
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.models.interfaces.PointerWrapper
17+
import Temporaries
18+
19+
predicate isUniquePointerDerefFunction(Function f) {
20+
exists(PointerWrapper wrapper |
21+
f = wrapper.getAnUnwrapperFunction() and
22+
// We only want unique pointers as the memory behind share pointers may still be
23+
// alive after the shared pointer is destroyed.
24+
wrapper.(Class).hasQualifiedName(["std", "bsl"], "unique_ptr")
25+
)
26+
}
27+
28+
from Call c
29+
where
30+
outlivesFullExpr(c) and
31+
not c.isFromUninstantiatedTemplate(_) and
32+
isUniquePointerDerefFunction(c.getTarget()) and
33+
isTemporary(c.getQualifier().getFullyConverted())
34+
select c,
35+
"The underlying unique pointer object is destroyed after the call to '" + c.getTarget() +
36+
"' returns."

‎cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsBad.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include <memory>
2+
std::unique_ptr<T> getUniquePointer();
3+
void work(const T*);
4+
5+
// BAD: the unique pointer is deallocated when `get` returns. So `work`
6+
// is given a pointer to invalid memory.
7+
void work_with_unique_ptr_bad() {
8+
const T* combined_string = getUniquePointer().get();
9+
work(combined_string);
10+
}

‎cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEndsGood.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include <memory>
2+
std::unique_ptr<T> getUniquePointer();
3+
void work(const T*);
4+
5+
// GOOD: the unique pointer outlives the call to `work`. So the pointer
6+
// obtainted from `get` is valid.
7+
void work_with_unique_ptr_good() {
8+
auto combined_string = getUniquePointer();
9+
work(combined_string.get());
10+
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
| test.cpp:156:15:156:15 | call to operator* | The underlying unique pointer object is destroyed after the call to 'operator*' returns. |
2+
| test.cpp:157:31:157:33 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
3+
| test.cpp:159:32:159:32 | call to operator-> | The underlying unique pointer object is destroyed after the call to 'operator->' returns. |
4+
| test.cpp:160:35:160:37 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
5+
| test.cpp:161:44:161:46 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
6+
| test.cpp:163:25:163:27 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
7+
| test.cpp:172:33:172:35 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
8+
| test.cpp:174:32:174:34 | call to get | The underlying unique pointer object is destroyed after the call to 'get' returns. |
9+
| test.cpp:176:11:176:11 | call to operator* | The underlying unique pointer object is destroyed after the call to 'operator*' returns. |

‎cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/UseOfUniquePtrAfterLifetimeEnds/UseOfUniquePointerAfterLifetimeEnds.qlref

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

0 commit comments

Comments
 (0)