]>
Commit | Line | Data |
---|---|---|
863bff75 FS |
1 | /** |
2 | * vi: sw=2 ts=2 et syntax=ql: | |
3 | * | |
4 | * Based on cpp/uninitialized-local. | |
5 | * | |
6 | * @name Potentially uninitialized local variable using the cleanup attribute | |
7 | * @description Running the cleanup handler on a possibly uninitialized variable | |
8 | * is generally a bad idea. | |
9 | * @id cpp/uninitialized-local-with-cleanup | |
10 | * @kind problem | |
11 | * @problem.severity error | |
12 | * @precision high | |
13 | * @tags security | |
14 | */ | |
15 | ||
16 | import cpp | |
17 | import semmle.code.cpp.controlflow.StackVariableReachability | |
18 | ||
af186821 FS |
19 | /** Auxiliary predicate: List cleanup functions we want to explicitly ignore |
20 | * since they don't do anything illegal even when the variable is uninitialized | |
21 | */ | |
22 | predicate cleanupFunctionDenyList(string fun) { | |
23 | fun = "erase_char" | |
24 | } | |
25 | ||
863bff75 FS |
26 | /** |
27 | * A declaration of a local variable using __attribute__((__cleanup__(x))) | |
28 | * that leaves the variable uninitialized. | |
29 | */ | |
30 | DeclStmt declWithNoInit(LocalVariable v) { | |
31 | result.getADeclaration() = v and | |
c8fec8bf | 32 | not v.hasInitializer() and |
863bff75 FS |
33 | /* The variable has __attribute__((__cleanup__(...))) set */ |
34 | v.getAnAttribute().hasName("cleanup") and | |
af186821 | 35 | /* Check if the cleanup function is not on a deny list */ |
c8fec8bf | 36 | not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText()) |
863bff75 FS |
37 | } |
38 | ||
39 | class UninitialisedLocalReachability extends StackVariableReachability { | |
40 | UninitialisedLocalReachability() { this = "UninitialisedLocal" } | |
41 | ||
42 | override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) } | |
43 | ||
44 | /* Note: _don't_ use the `useOfVarActual()` predicate here (and a couple of lines | |
45 | * below), as it assumes that the callee always modifies the variable if | |
46 | * it's passed to the function. | |
47 | * | |
48 | * i.e.: | |
49 | * _cleanup_free char *x; | |
50 | * fun(&x); | |
51 | * puts(x); | |
52 | * | |
53 | * `useOfVarActual()` won't treat this an an uninitialized read even if the callee | |
54 | * doesn't modify the argument, however, `useOfVar()` will | |
55 | */ | |
56 | override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVar(v, node) } | |
57 | ||
58 | override predicate isBarrier(ControlFlowNode node, StackVariable v) { | |
59 | // only report the _first_ possibly uninitialized use | |
60 | useOfVar(v, node) or | |
c8fec8bf FS |
61 | ( |
62 | /* If there's an return statement somewhere between the variable declaration | |
63 | * and a possible definition, don't accept is as a valid initialization. | |
64 | * | |
65 | * E.g.: | |
66 | * _cleanup_free_ char *x; | |
67 | * ... | |
68 | * if (...) | |
69 | * return; | |
70 | * ... | |
71 | * x = malloc(...); | |
72 | * | |
73 | * is not a valid initialization, since we might return from the function | |
74 | * _before_ the actual iniitialization (emphasis on _might_, since we | |
75 | * don't know if the return statement might ever evaluate to true). | |
76 | */ | |
77 | definitionBarrier(v, node) and | |
78 | not exists(ReturnStmt rs | | |
79 | /* The attribute check is "just" a complexity optimization */ | |
80 | v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") | | |
81 | rs.getLocation().isBefore(node.getLocation()) | |
82 | ) | |
83 | ) | |
863bff75 FS |
84 | } |
85 | } | |
86 | ||
87 | pragma[noinline] | |
88 | predicate containsInlineAssembly(Function f) { exists(AsmStmt s | s.getEnclosingFunction() = f) } | |
89 | ||
90 | /** | |
91 | * Auxiliary predicate: List common exceptions or false positives | |
92 | * for this check to exclude them. | |
93 | */ | |
94 | VariableAccess commonException() { | |
95 | // If the uninitialized use we've found is in a macro expansion, it's | |
96 | // typically something like va_start(), and we don't want to complain. | |
97 | result.getParent().isInMacroExpansion() | |
98 | or | |
99 | result.getParent() instanceof BuiltInOperation | |
100 | or | |
101 | // Finally, exclude functions that contain assembly blocks. It's | |
102 | // anyone's guess what happens in those. | |
103 | containsInlineAssembly(result.getEnclosingFunction()) | |
104 | } | |
105 | ||
106 | from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va | |
107 | where | |
108 | r.reaches(_, v, va) and | |
109 | not va = commonException() | |
110 | select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName() |