diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll index de1c3389be00..1162c09b0b64 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll @@ -27,10 +27,12 @@ private class StandardDeallocationFunction extends DeallocationFunction { or this.hasGlobalOrStdName([ // --- Windows Memory Management for Windows Drivers - "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeMdl", "IoFreeWorkItem", "IoFreeErrorLogEntry", - "MmFreeContiguousMemory", "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory", - "MmFreeMappingAddress", "MmFreePagesFromMdl", "MmUnmapReservedMapping", - "MmUnmapLockedPages", + "ExFreePool", "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeIrp", "IoFreeMdl", + "IoFreeErrorLogEntry", "IoFreeWorkItem", "MmFreeContiguousMemory", + "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory", "MmFreeMappingAddress", + "MmFreePagesFromMdl", "MmUnmapReservedMapping", "MmUnmapLockedPages", + "NdisFreeGenericObject", "NdisFreeMemory", "NdisFreeMemoryWithTag", "NdisFreeMdl", + "NdisFreeNetBufferListPool", "NdisFreeNetBufferPool", // --- Windows Global / Local legacy allocation "LocalFree", "GlobalFree", "LocalReAlloc", "GlobalReAlloc", // --- Windows System Services allocation @@ -47,6 +49,7 @@ private class StandardDeallocationFunction extends DeallocationFunction { this.hasGlobalOrStdName([ // --- Windows Memory Management for Windows Drivers "ExFreeToLookasideListEx", "ExFreeToPagedLookasideList", "ExFreeToNPagedLookasideList", + "NdisFreeMemoryWithTagPriority", "StorPortFreeMdl", "StorPortFreePool", // --- NetBSD pool manager "pool_put", "pool_cache_put" ]) and diff --git a/cpp/ql/src/Critical/FlowAfterFree.qll b/cpp/ql/lib/semmle/code/cpp/security/flowafterfree/FlowAfterFree.qll similarity index 54% rename from cpp/ql/src/Critical/FlowAfterFree.qll rename to cpp/ql/lib/semmle/code/cpp/security/flowafterfree/FlowAfterFree.qll index caca108f4b26..3172b6e35622 100644 --- a/cpp/ql/src/Critical/FlowAfterFree.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/flowafterfree/FlowAfterFree.qll @@ -1,20 +1,10 @@ -import cpp -import semmle.code.cpp.dataflow.new.DataFlow -private import semmle.code.cpp.ir.IR - /** - * Signature for a predicate that holds if `n.asExpr() = e` and `n` is a sink in - * the `FlowFromFreeConfig` module. + * General library for finding flow from a pointer being freed to a user-specified sink */ -private signature predicate isSinkSig(DataFlow::Node n, Expr e); -/** - * Holds if `dealloc` is a deallocation expression and `e` is an expression such - * that `isFree(_, e)` holds for some `isFree` predicate satisfying `isSinkSig`, - * and this source-sink pair should be excluded from the analysis. - */ -bindingset[dealloc, e] -private signature predicate isExcludedSig(DeallocationExpr dealloc, Expr e); +import cpp +import semmle.code.cpp.dataflow.new.DataFlow +private import semmle.code.cpp.ir.IR /** * Holds if `(b1, i1)` strictly post-dominates `(b2, i2)` @@ -38,6 +28,31 @@ predicate strictlyDominates(IRBlock b1, int i1, IRBlock b2, int i2) { b1.strictlyDominates(b2) } +/** + * The signature for a module that is used to specify the inputs to the `FlowFromFree` module. + */ +signature module FlowFromFreeParamSig { + /** + * Holds if `n.asExpr() = e` and `n` is a sink in the `FlowFromFreeConfig` + * module. + */ + predicate isSink(DataFlow::Node n, Expr e); + + /** + * Holds if `dealloc` is a deallocation expression and `e` is an expression such + * that `isFree(_, e)` holds for some `isFree` predicate satisfying `isSinkSig`, + * and this source-sink pair should be excluded from the analysis. + */ + bindingset[dealloc, e] + predicate isExcluded(DeallocationExpr dealloc, Expr e); + + /** + * Holds if `sink` should be considered a `sink` when the source of flow is `source`. + */ + bindingset[source, sink] + default predicate sourceSinkIsRelated(DataFlow::Node source, DataFlow::Node sink) { any() } +} + /** * Constructs a `FlowFromFreeConfig` module that can be used to find flow between * a pointer being freed by some deallocation function, and a user-specified sink. @@ -47,8 +62,8 @@ predicate strictlyDominates(IRBlock b1, int i1, IRBlock b2, int i2) { * 1. The source dominates the sink, or * 2. The sink post-dominates the source. */ -module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> { - module FlowFromFreeConfig implements DataFlow::StateConfigSig { +module FlowFromFree<FlowFromFreeParamSig P> { + private module FlowFromFreeConfig implements DataFlow::StateConfigSig { class FlowState instanceof Expr { FlowState() { isFree(_, _, this, _) } @@ -59,20 +74,12 @@ module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> { pragma[inline] predicate isSink(DataFlow::Node sink, FlowState state) { - exists( - Expr e, DataFlow::Node source, IRBlock b1, int i1, IRBlock b2, int i2, - DeallocationExpr dealloc - | - isASink(sink, e) and + exists(Expr e, DataFlow::Node source, DeallocationExpr dealloc | + P::isSink(sink, e) and isFree(source, _, state, dealloc) and e != state and - source.hasIndexInBlock(b1, i1) and - sink.hasIndexInBlock(b2, i2) and - not isExcluded(dealloc, e) - | - strictlyDominates(b1, i1, b2, i2) - or - strictlyPostDominates(b2, i2, b1, i1) + not P::isExcluded(dealloc, e) and + P::sourceSinkIsRelated(source, sink) ) } @@ -127,3 +134,38 @@ predicate isExFreePoolCall(FunctionCall fc, Expr e) { fc.getTarget().hasGlobalName("ExFreePool") ) } + +/** + * Holds if either `source` strictly dominates `sink`, or `sink` strictly + * post-dominates `source`. + */ +bindingset[source, sink] +predicate defaultSourceSinkIsRelated(DataFlow::Node source, DataFlow::Node sink) { + exists(IRBlock b1, int i1, IRBlock b2, int i2 | + source.hasIndexInBlock(b1, i1) and + sink.hasIndexInBlock(b2, i2) + | + strictlyDominates(b1, i1, b2, i2) + or + strictlyPostDominates(b2, i2, b1, i1) + ) +} + +/** + * `dealloc1` is a deallocation expression, `e` is an expression that dereferences a + * pointer, and the `(dealloc1, e)` pair should be excluded by the `FlowFromFree` library. + * + * Note that `e` is not necessarily the expression deallocated by `dealloc1`. It will + * be bound to the second deallocation as identified by the `FlowFromFree` library. + * + * From https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-mmfreepagesfrommdl: + * "After calling MmFreePagesFromMdl, the caller must also call ExFreePool + * to release the memory that was allocated for the MDL structure." + */ +bindingset[dealloc1, e] +predicate isExcludedMmFreePageFromMdl(DeallocationExpr dealloc1, Expr e) { + exists(DeallocationExpr dealloc2 | isFree(_, _, e, dealloc2) | + dealloc1.(FunctionCall).getTarget().hasGlobalName("MmFreePagesFromMdl") and + isExFreePoolCall(dealloc2, _) + ) +} diff --git a/cpp/ql/lib/semmle/code/cpp/security/flowafterfree/UseAfterFree.qll b/cpp/ql/lib/semmle/code/cpp/security/flowafterfree/UseAfterFree.qll new file mode 100644 index 000000000000..bea0b73b8748 --- /dev/null +++ b/cpp/ql/lib/semmle/code/cpp/security/flowafterfree/UseAfterFree.qll @@ -0,0 +1,159 @@ +/** + * General library for tracing Use After Free vulnerabilities. + */ + +import cpp +private import semmle.code.cpp.security.flowafterfree.FlowAfterFree +private import semmle.code.cpp.ir.IR + +/** + * 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" + ) +} + +/** + * Holds if `e` is a use. A use is a pointer dereference or a + * parameter to a call with no function definition. + * Uses in deallocation expressions (e.g., free) are excluded. + * Default isUse definition for an expression. + */ +predicate isUse0(Expr e) { + not isFree(_, _, e, _) and + ( + // TODO: use DirectDefereferencedByOperation in Dereferenced.qll + e = any(PointerDereferenceExpr pde).getOperand() + or + e = any(PointerFieldAccess pfa).getQualifier() + or + e = any(ArrayExpr ae).getArrayBase() + or + e = any(Call call).getQualifier() + or + // Assume any function without a body will dereference the pointer + exists(int i, Call call, Function f | + e = 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) + ) + ) +} + +private module ParameterSinks { + import semmle.code.cpp.ir.ValueNumbering + + private predicate flowsToUse(DataFlow::Node n) { + isUse0(n.asExpr()) + or + exists(DataFlow::Node succ | + flowsToUse(succ) and + DataFlow::localFlowStep(n, succ) + ) + } + + 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) + ) + ) + } + + private predicate step(DataFlow::Node n1, DataFlow::Node n2) { + flowsFromParam(n1) and + flowsFromParam(n2) and + DataFlow::localFlowStep(n1, n2) + } + + 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.asExpr()) + } + + 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, pragma[only_bind_into](i1)) and + sink.hasIndexInBlock(b2, pragma[only_bind_into](i2)) and + strictlyPostDominates(b2, i2, b1, i1) + ) + } + + private CallInstruction getAnAlwaysReachedCallInstruction() { + exists(IRFunction f | result.getBlock().postDominates(f.getEntryBlock())) + } + + pragma[nomagic] + private predicate callHasTargetAndArgument(Function f, int i, Instruction argument) { + exists(CallInstruction call | + call.getStaticCallTarget() = f and + call.getArgument(i) = argument and + call = getAnAlwaysReachedCallInstruction() + ) + } + + pragma[nomagic] + private predicate initializeParameterInFunction(Function f, int i) { + exists(InitializeParameterInstruction init | + pragma[only_bind_out](init.getEnclosingFunction()) = f and + init.hasIndex(i) and + init = getAnAlwaysDereferencedParameter() + ) + } + + pragma[nomagic] + private predicate alwaysDereferencedArgumentHasValueNumber(ValueNumber vn) { + exists(int i, Function f, Instruction argument | + callHasTargetAndArgument(f, i, argument) and + initializeParameterInFunction(pragma[only_bind_into](f), pragma[only_bind_into](i)) and + vn.getAnInstruction() = argument + ) + } + + InitializeParameterInstruction getAnAlwaysDereferencedParameter() { + result = getAnAlwaysDereferencedParameter0() + or + exists(ValueNumber vn | + alwaysDereferencedArgumentHasValueNumber(vn) and + vn.getAnInstruction() = result + ) + } +} + +private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon + +/** + * Holds if `n` represents the expression `e`, and `e` is a pointer that is + * guaranteed to be dereferenced (either because it's an operand of a + * dereference operation, or because it's an argument to a function that + * always dereferences the parameter). + */ +predicate isUse(DataFlow::Node n, Expr e) { + isUse0(e) and n.asExpr() = e + or + exists(CallInstruction call, InitializeParameterInstruction init | + n.asOperand().getDef().getUnconvertedResultExpression() = e and + pragma[only_bind_into](init) = ParameterSinks::getAnAlwaysDereferencedParameter() and + viableParamArg(call, DataFlow::instructionNode(init), n) and + pragma[only_bind_out](init.getEnclosingFunction()) = + pragma[only_bind_out](call.getStaticCallTarget()) + ) +} diff --git a/cpp/ql/src/Critical/DoubleFree.ql b/cpp/ql/src/Critical/DoubleFree.ql index 92cdb0807e4d..802c81fd050b 100644 --- a/cpp/ql/src/Critical/DoubleFree.ql +++ b/cpp/ql/src/Critical/DoubleFree.ql @@ -13,7 +13,7 @@ import cpp import semmle.code.cpp.dataflow.new.DataFlow -import FlowAfterFree +import semmle.code.cpp.security.flowafterfree.FlowAfterFree import DoubleFree::PathGraph /** @@ -22,26 +22,15 @@ import DoubleFree::PathGraph */ predicate isFree(DataFlow::Node n, Expr e) { isFree(_, n, e, _) } -/** - * `dealloc1` is a deallocation expression and `e` is an expression such - * that is deallocated by a deallocation expression, and the `(dealloc1, e)` pair - * should be excluded by the `FlowFromFree` library. - * - * Note that `e` is not necessarily the expression deallocated by `dealloc1`. It will - * be bound to the second deallocation as identified by the `FlowFromFree` library. - */ -bindingset[dealloc1, e] -predicate isExcludeFreePair(DeallocationExpr dealloc1, Expr e) { - exists(DeallocationExpr dealloc2 | isFree(_, _, e, dealloc2) | - dealloc1.(FunctionCall).getTarget().hasGlobalName("MmFreePagesFromMdl") and - // From https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-mmfreepagesfrommdl: - // "After calling MmFreePagesFromMdl, the caller must also call ExFreePool - // to release the memory that was allocated for the MDL structure." - isExFreePoolCall(dealloc2, _) - ) +module DoubleFreeParam implements FlowFromFreeParamSig { + predicate isSink = isFree/2; + + predicate isExcluded = isExcludedMmFreePageFromMdl/2; + + predicate sourceSinkIsRelated = defaultSourceSinkIsRelated/2; } -module DoubleFree = FlowFromFree<isFree/2, isExcludeFreePair/2>; +module DoubleFree = FlowFromFree<DoubleFreeParam>; from DoubleFree::PathNode source, DoubleFree::PathNode sink, DeallocationExpr dealloc, Expr e2 where diff --git a/cpp/ql/src/Critical/UseAfterFree.ql b/cpp/ql/src/Critical/UseAfterFree.ql index 4b27369074c5..435a0993d537 100644 --- a/cpp/ql/src/Critical/UseAfterFree.ql +++ b/cpp/ql/src/Critical/UseAfterFree.ql @@ -14,170 +14,25 @@ import cpp import semmle.code.cpp.dataflow.new.DataFlow import semmle.code.cpp.ir.IR -import FlowAfterFree -import UseAfterFree::PathGraph +import semmle.code.cpp.security.flowafterfree.FlowAfterFree +import semmle.code.cpp.security.flowafterfree.UseAfterFree +import UseAfterFreeTrace::PathGraph -/** - * 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" - ) -} - -predicate isUse0(Expr e) { - not isFree(_, _, e, _) and - ( - e = any(PointerDereferenceExpr pde).getOperand() - or - e = any(PointerFieldAccess pfa).getQualifier() - or - e = any(ArrayExpr ae).getArrayBase() - or - e = any(Call call).getQualifier() - or - // Assume any function without a body will dereference the pointer - exists(int i, Call call, Function f | - e = 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) - ) - ) -} - -module ParameterSinks { - import semmle.code.cpp.ir.ValueNumbering - - predicate flowsToUse(DataFlow::Node n) { - isUse0(n.asExpr()) - or - exists(DataFlow::Node succ | - flowsToUse(succ) and - DataFlow::localFlowStep(n, succ) - ) - } - - 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) - ) - ) - } - - private predicate step(DataFlow::Node n1, DataFlow::Node n2) { - flowsFromParam(n1) and - flowsFromParam(n2) and - DataFlow::localFlowStep(n1, n2) - } - - 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.asExpr()) - } +module UseAfterFreeParam implements FlowFromFreeParamSig { + predicate isSink = isUse/2; - 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, pragma[only_bind_into](i1)) and - sink.hasIndexInBlock(b2, pragma[only_bind_into](i2)) and - strictlyPostDominates(b2, i2, b1, i1) - ) - } + predicate isExcluded = isExcludedMmFreePageFromMdl/2; - private CallInstruction getAnAlwaysReachedCallInstruction() { - exists(IRFunction f | result.getBlock().postDominates(f.getEntryBlock())) - } - - pragma[nomagic] - private predicate callHasTargetAndArgument(Function f, int i, Instruction argument) { - exists(CallInstruction call | - call.getStaticCallTarget() = f and - call.getArgument(i) = argument and - call = getAnAlwaysReachedCallInstruction() - ) - } - - pragma[nomagic] - private predicate initializeParameterInFunction(Function f, int i) { - exists(InitializeParameterInstruction init | - pragma[only_bind_out](init.getEnclosingFunction()) = f and - init.hasIndex(i) and - init = getAnAlwaysDereferencedParameter() - ) - } - - pragma[nomagic] - private predicate alwaysDereferencedArgumentHasValueNumber(ValueNumber vn) { - exists(int i, Function f, Instruction argument | - callHasTargetAndArgument(f, i, argument) and - initializeParameterInFunction(pragma[only_bind_into](f), pragma[only_bind_into](i)) and - vn.getAnInstruction() = argument - ) - } - - InitializeParameterInstruction getAnAlwaysDereferencedParameter() { - result = getAnAlwaysDereferencedParameter0() - or - exists(ValueNumber vn | - alwaysDereferencedArgumentHasValueNumber(vn) and - vn.getAnInstruction() = result - ) - } + predicate sourceSinkIsRelated = defaultSourceSinkIsRelated/2; } -module IsUse { - private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon - - predicate isUse(DataFlow::Node n, Expr e) { - isUse0(e) and n.asExpr() = e - or - exists(CallInstruction call, InitializeParameterInstruction init | - n.asOperand().getDef().getUnconvertedResultExpression() = e and - pragma[only_bind_into](init) = ParameterSinks::getAnAlwaysDereferencedParameter() and - viableParamArg(call, DataFlow::instructionNode(init), n) and - pragma[only_bind_out](init.getEnclosingFunction()) = - pragma[only_bind_out](call.getStaticCallTarget()) - ) - } -} - -import IsUse - -/** - * `dealloc1` is a deallocation expression, `e` is an expression that dereferences a - * pointer, and the `(dealloc1, e)` pair should be excluded by the `FlowFromFree` library. - */ -bindingset[dealloc1, e] -predicate isExcludeFreeUsePair(DeallocationExpr dealloc1, Expr e) { - // From https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-mmfreepagesfrommdl: - // "After calling MmFreePagesFromMdl, the caller must also call ExFreePool - // to release the memory that was allocated for the MDL structure." - dealloc1.(FunctionCall).getTarget().hasGlobalName("MmFreePagesFromMdl") and - isExFreePoolCall(_, e) -} +import UseAfterFreeParam -module UseAfterFree = FlowFromFree<isUse/2, isExcludeFreeUsePair/2>; +module UseAfterFreeTrace = FlowFromFree<UseAfterFreeParam>; -from UseAfterFree::PathNode source, UseAfterFree::PathNode sink, DeallocationExpr dealloc +from UseAfterFreeTrace::PathNode source, UseAfterFreeTrace::PathNode sink, DeallocationExpr dealloc where - UseAfterFree::flowPath(source, sink) and + UseAfterFreeTrace::flowPath(source, sink) and isFree(source.getNode(), _, _, dealloc) select sink.getNode(), source, sink, "Memory may have been previously freed by $@.", dealloc, dealloc.toString() diff --git a/cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected b/cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected index f5a3553ce542..14b04cc5c2dd 100644 --- a/cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected +++ b/cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected @@ -89,6 +89,7 @@ | test_free.cpp:216:10:216:10 | a | | test_free.cpp:220:10:220:10 | a | | test_free.cpp:227:24:227:45 | memory_descriptor_list | +| test_free.cpp:228:16:228:37 | memory_descriptor_list | | test_free.cpp:233:14:233:15 | * ... | | test_free.cpp:239:14:239:15 | * ... | | test_free.cpp:245:10:245:11 | * ... |