diff --git a/cpp/ql/src/Critical/UseAfterFree.ql b/cpp/ql/src/Critical/UseAfterFree.ql index 3c67c99ef3c..4e7bca8f331 100644 --- a/cpp/ql/src/Critical/UseAfterFree.ql +++ b/cpp/ql/src/Critical/UseAfterFree.ql @@ -1,7 +1,8 @@ /** * @name Potential use after free * @description An allocated memory block is used after it has been freed. Behavior in such cases is undefined and can cause memory corruption. - * @kind problem + * @kind path-problem + * @precision medium * @id cpp/use-after-free * @problem.severity warning * @security-severity 9.3 @@ -11,56 +12,131 @@ */ import cpp -import semmle.code.cpp.controlflow.StackVariableReachability +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.ir.IR +import FlowAfterFree +import UseAfterFree::PathGraph -/** `e` is an expression that frees the memory pointed to by `v`. */ -predicate isFreeExpr(Expr e, StackVariable v) { - exists(VariableAccess va | va.getTarget() = v | - exists(FunctionCall fc | fc = e | - fc.getTarget().hasGlobalOrStdName("free") and - va = fc.getArgument(0) - ) - or - e.(DeleteExpr).getExpr() = va - or - e.(DeleteArrayExpr).getExpr() = va +/** + * Holds if `call` is a call to a function that obviously + * doesn't dereference its `i`'th argument. + */ +private predicate externalCallNeverDereferences(FormattingFunctionCall call, int arg) { + exists(int formatArg | + pragma[only_bind_out](call.getFormatArgument(formatArg)) = + pragma[only_bind_out](call.getArgument(arg)) and + call.getFormat().(FormatLiteral).getConvSpec(formatArg) != "%s" ) } -/** `e` is an expression that (may) dereference `v`. */ -predicate isDerefExpr(Expr e, StackVariable v) { - v.getAnAccess() = e and dereferenced(e) - or - isDerefByCallExpr(_, _, e, v) +predicate isUse0(DataFlow::Node n, Expr e) { + e = n.asExpr() and + not isFree(_, e, _) and + ( + e = any(PointerDereferenceExpr pde).getOperand() + or + e = any(PointerFieldAccess pfa).getQualifier() + or + e = any(ArrayExpr ae).getArrayBase() + or + // Assume any function without a body will dereference the pointer + exists(int i, Call call, Function f | + n.asExpr() = call.getArgument(i) and + f = call.getTarget() and + not f.hasEntryPoint() and + // Exclude known functions we know won't dereference the pointer. + // For example, a call such as `printf("%p", myPointer)`. + not externalCallNeverDereferences(call, i) + ) + ) } -/** - * `va` is passed by value as (part of) the `i`th argument in - * call `c`. The target function is either a library function - * or a source code function that dereferences the relevant - * parameter. - */ -predicate isDerefByCallExpr(Call c, int i, VariableAccess va, StackVariable v) { - v.getAnAccess() = va and - va = c.getAnArgumentSubExpr(i) and - not c.passesByReference(i, va) and - (c.getTarget().hasEntryPoint() implies isDerefExpr(_, c.getTarget().getParameter(i))) -} +module ParameterSinks { + import semmle.code.cpp.ir.ValueNumbering -class UseAfterFreeReachability extends StackVariableReachability { - UseAfterFreeReachability() { this = "UseAfterFree" } + predicate flowsToUse(DataFlow::Node n) { + isUse0(n, _) + or + exists(DataFlow::Node succ | + flowsToUse(succ) and + DataFlow::localFlowStep(n, succ) + ) + } - override predicate isSource(ControlFlowNode node, StackVariable v) { isFreeExpr(node, v) } + private predicate flowsFromParam(DataFlow::Node n) { + flowsToUse(n) and + ( + n.asParameter().getUnspecifiedType() instanceof PointerType + or + exists(DataFlow::Node prev | + flowsFromParam(prev) and + DataFlow::localFlowStep(prev, n) + ) + ) + } - override predicate isSink(ControlFlowNode node, StackVariable v) { isDerefExpr(node, v) } + private predicate step(DataFlow::Node n1, DataFlow::Node n2) { + flowsFromParam(n1) and + flowsFromParam(n2) and + DataFlow::localFlowStep(n1, n2) + } - override predicate isBarrier(ControlFlowNode node, StackVariable v) { - definitionBarrier(v, node) or - isFreeExpr(node, v) + private predicate paramToUse(DataFlow::Node n1, DataFlow::Node n2) = fastTC(step/2)(n1, n2) + + private predicate hasFlow( + DataFlow::Node source, InitializeParameterInstruction init, DataFlow::Node sink + ) { + pragma[only_bind_out](source.asParameter()) = pragma[only_bind_out](init.getParameter()) and + paramToUse(source, sink) and + isUse0(sink, _) + } + + private InitializeParameterInstruction getAnAlwaysDereferencedParameter0() { + exists(DataFlow::Node source, DataFlow::Node sink, IRBlock b1, int i1, IRBlock b2, int i2 | + hasFlow(pragma[only_bind_into](source), result, pragma[only_bind_into](sink)) and + source.hasIndexInBlock(b1, i1) and + sink.hasIndexInBlock(b2, i2) and + strictlyPostDominates(b2, i2, b1, i1) + ) + } + + private CallInstruction getAnAlwaysReachedCallInstruction(IRFunction f) { + result.getBlock().postDominates(f.getEntryBlock()) + } + + InitializeParameterInstruction getAnAlwaysDereferencedParameter() { + result = getAnAlwaysDereferencedParameter0() + or + exists(CallInstruction call, int i, InitializeParameterInstruction p | + pragma[only_bind_out](call.getStaticCallTarget()) = + pragma[only_bind_out](p.getEnclosingFunction()) and + p.hasIndex(i) and + p = getAnAlwaysDereferencedParameter() and + result = valueNumber(call.getArgument(i)).getAnInstruction() and + call = getAnAlwaysReachedCallInstruction(_) + ) } } -from UseAfterFreeReachability r, StackVariable v, Expr free, Expr e -where r.reaches(free, v, e) -select e, "Memory pointed to by '" + v.getName().toString() + "' may have $@.", free, - "been previously freed" +predicate isUse(DataFlow::Node n, Expr e) { + isUse0(n, e) + or + exists(CallInstruction call, int i, InitializeParameterInstruction init | + n.asOperand().getDef().getUnconvertedResultExpression() = e and + init = ParameterSinks::getAnAlwaysDereferencedParameter() and + call.getArgumentOperand(i) = n.asOperand() and + init.hasIndex(i) and + init.getEnclosingFunction() = call.getStaticCallTarget() + ) +} + +predicate excludeNothing(DeallocationExpr dealloc, Expr e) { none() } + +module UseAfterFree = FlowFromFree; + +from UseAfterFree::PathNode source, UseAfterFree::PathNode sink, DeallocationExpr dealloc +where + UseAfterFree::flowPath(source, sink) and + isFree(source.getNode(), _, dealloc) +select sink.getNode(), source, sink, "Memory may have been previously freed by $@.", dealloc, + dealloc.toString()