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
16import cpp
17import semmle.code.cpp.controlflow.StackVariableReachability
18
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  */
22predicate cleanupFunctionDenyList(string fun) {
23  fun = "erase_char"
24}
25
26/**
27 * A declaration of a local variable using __attribute__((__cleanup__(x)))
28 * that leaves the variable uninitialized.
29 */
30DeclStmt declWithNoInit(LocalVariable v) {
31  result.getADeclaration() = v and
32  not v.hasInitializer() and
33  /* The variable has __attribute__((__cleanup__(...))) set */
34  v.getAnAttribute().hasName("cleanup") and
35  /* Check if the cleanup function is not on a deny list */
36  not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText())
37}
38
39class 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
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    )
84  }
85}
86
87pragma[noinline]
88predicate 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 */
94VariableAccess 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
106from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
107where
108  r.reaches(_, v, va) and
109  not va = commonException()
110select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName()
111