-
Notifications
You must be signed in to change notification settings - Fork 1.5k
/
ReturnStackAllocatedMemory.ql
94 lines (86 loc) · 3.55 KB
/
ReturnStackAllocatedMemory.ql
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
/**
* @name Returning stack-allocated memory
* @description A function returns a pointer to a stack-allocated region of
* memory. This memory is deallocated at the end of the function,
* which may lead the caller to dereference a dangling pointer.
* @kind path-problem
* @id cpp/return-stack-allocated-memory
* @problem.severity warning
* @security-severity 9.3
* @precision high
* @tags reliability
* security
* external/cwe/cwe-825
*/
import cpp
import semmle.code.cpp.ir.IR
import semmle.code.cpp.ir.dataflow.MustFlow
import PathGraph
/** Holds if `f` has a name that we interpret as evidence of intentionally returning the value of the stack pointer. */
predicate intentionallyReturnsStackPointer(Function f) {
f.getName().toLowerCase().matches(["%stack%", "%sp%"])
}
class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
ReturnStackAllocatedMemoryConfig() { this = "ReturnStackAllocatedMemoryConfig" }
override predicate isSource(Instruction source) {
// Holds if `source` is a node that represents the use of a stack variable
exists(VariableAddressInstruction var, Function func |
var = source and
func = source.getEnclosingFunction() and
var.getAstVariable() instanceof StackVariable and
// Pointer-to-member types aren't properly handled in the dbscheme.
not var.getResultType() instanceof PointerToMemberType and
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = func and
not intentionallyReturnsStackPointer(func)
)
}
override predicate isSink(Operand sink) {
// Holds if `sink` is a node that represents the `StoreInstruction` that is subsequently used in
// a `ReturnValueInstruction`.
// We use the `StoreInstruction` instead of the instruction that defines the
// `ReturnValueInstruction`'s source value operand because the former has better location information.
exists(StoreInstruction store |
store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
IRReturnVariable and
sink = store.getSourceValueOperand()
)
}
// We disable flow into callables in this query as we'd otherwise get a result on this piece of code:
// ```cpp
// int* id(int* px) {
// return px; // this returns the local variable `x`, but it's fine as the local variable isn't declared in this scope.
// }
// void f() {
// int x;
// int* px = id(&x);
// }
// ```
override predicate allowInterproceduralFlow() { none() }
/**
* This configuration intentionally conflates addresses of fields and their object, and pointer offsets
* with their base pointer as this allows us to detect cases where an object's address flows to a
* return statement via a field. For example:
*
* ```cpp
* struct S { int x, y };
* int* test() {
* S s;
* return &s.x; // BAD: &s.x is an address of a variable on the stack.
* }
* ```
*/
override predicate isAdditionalFlowStep(Operand node1, Instruction node2) {
node2.(FieldAddressInstruction).getObjectAddressOperand() = node1
or
node2.(PointerOffsetInstruction).getLeftOperand() = node1
}
}
from
MustFlowPathNode source, MustFlowPathNode sink, VariableAddressInstruction var,
ReturnStackAllocatedMemoryConfig conf
where
conf.hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and
source.getInstruction() = var
select sink.getInstruction(), source, sink, "May return stack-allocated memory from $@.",
var.getAst(), var.getAst().toString()