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
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
|
/**
* vi: sw=2 ts=2 et syntax=ql:
*
* Based on cpp/uninitialized-local.
*
* @name Potentially uninitialized local variable using the cleanup attribute
* @description Running the cleanup handler on a possibly uninitialized variable
* is generally a bad idea.
* @id cpp/uninitialized-local-with-cleanup
* @kind problem
* @problem.severity error
* @precision high
* @tags security
*/
import cpp
import semmle.code.cpp.controlflow.StackVariableReachability
/** Auxiliary predicate: List cleanup functions we want to explicitly ignore
* since they don't do anything illegal even when the variable is uninitialized
*/
predicate cleanupFunctionDenyList(string fun) {
fun = "erase_char"
}
/**
* A declaration of a local variable using __attribute__((__cleanup__(x)))
* that leaves the variable uninitialized.
*/
DeclStmt declWithNoInit(LocalVariable v) {
result.getADeclaration() = v and
not v.hasInitializer() and
/* The variable has __attribute__((__cleanup__(...))) set */
v.getAnAttribute().hasName("cleanup") and
/* Check if the cleanup function is not on a deny list */
not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText())
}
class UninitialisedLocalReachability extends StackVariableReachability {
UninitialisedLocalReachability() { this = "UninitialisedLocal" }
override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) }
/* Note: _don't_ use the `useOfVarActual()` predicate here (and a couple of lines
* below), as it assumes that the callee always modifies the variable if
* it's passed to the function.
*
* i.e.:
* _cleanup_free char *x;
* fun(&x);
* puts(x);
*
* `useOfVarActual()` won't treat this an an uninitialized read even if the callee
* doesn't modify the argument, however, `useOfVar()` will
*/
override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVar(v, node) }
override predicate isBarrier(ControlFlowNode node, StackVariable v) {
// only report the _first_ possibly uninitialized use
useOfVar(v, node) or
(
/* If there's an return statement somewhere between the variable declaration
* and a possible definition, don't accept is as a valid initialization.
*
* E.g.:
* _cleanup_free_ char *x;
* ...
* if (...)
* return;
* ...
* x = malloc(...);
*
* is not a valid initialization, since we might return from the function
* _before_ the actual iniitialization (emphasis on _might_, since we
* don't know if the return statement might ever evaluate to true).
*/
definitionBarrier(v, node) and
not exists(ReturnStmt rs |
/* The attribute check is "just" a complexity optimization */
v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") |
rs.getLocation().isBefore(node.getLocation())
)
)
}
}
pragma[noinline]
predicate containsInlineAssembly(Function f) { exists(AsmStmt s | s.getEnclosingFunction() = f) }
/**
* Auxiliary predicate: List common exceptions or false positives
* for this check to exclude them.
*/
VariableAccess commonException() {
// If the uninitialized use we've found is in a macro expansion, it's
// typically something like va_start(), and we don't want to complain.
result.getParent().isInMacroExpansion()
or
result.getParent() instanceof BuiltInOperation
or
// Finally, exclude functions that contain assembly blocks. It's
// anyone's guess what happens in those.
containsInlineAssembly(result.getEnclosingFunction())
}
from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
where
r.reaches(_, v, va) and
not va = commonException()
select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName()
|
