mirror of
https://github.com/systemd/systemd
synced 2024-07-09 04:26:06 +00:00
3f92dc2fd4
This erase_obj() machinery looks like voodoo and creates an awful lot of noise as soon as we get back to building with -O0. We can do this in a more simple way by introducing a struct that holds the information we need on cleanup. When building with optimization enabled, all this gets inlined and the eraser vanishes.
111 lines
3.8 KiB
Plaintext
111 lines
3.8 KiB
Plaintext
/**
|
|
* 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 as 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 a 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 initialization (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()
|