From ebb989962c17c1533cb1d078dc1f5d45e8214898 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 9 Dec 2025 16:17:46 +0100 Subject: [PATCH 1/4] Guards: Generalise ValidationWrapper to support GuardValue-based BarrierGuards. --- .../semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll | 4 ++-- .../lib/semmle/code/java/dataflow/internal/SsaImpl.qll | 6 ++++-- shared/controlflow/codeql/controlflow/Guards.qll | 10 +++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll index 285e0dc8419e..32ce89e26742 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll @@ -1051,12 +1051,12 @@ module BarrierGuardWithIntParam { } private predicate guardChecksInstr( - IRGuards::Guards_v1::Guard g, IRGuards::GuardsInput::Expr instr, boolean branch, + IRGuards::Guards_v1::Guard g, IRGuards::GuardsInput::Expr instr, IRGuards::GuardValue gv, int indirectionIndex ) { exists(Node node | nodeHasInstruction(node, instr, indirectionIndex) and - guardChecksNode(g, node, branch, indirectionIndex) + guardChecksNode(g, node, gv.asBooleanValue(), indirectionIndex) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 624f82fd341d..079fad797d8f 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -568,8 +568,10 @@ private module Cached { cached // nothing is actually cached module BarrierGuard { - private predicate guardChecksAdjTypes(Guards::Guards_v3::Guard g, Expr e, boolean branch) { - guardChecks(g, e, branch) + private predicate guardChecksAdjTypes( + Guards::Guards_v3::Guard g, Expr e, Guards::GuardValue gv + ) { + guardChecks(g, e, gv.asBooleanValue()) } private predicate guardChecksWithWrappers( diff --git a/shared/controlflow/codeql/controlflow/Guards.qll b/shared/controlflow/codeql/controlflow/Guards.qll index c0d07278b9a8..0e5cf25fecbf 100644 --- a/shared/controlflow/codeql/controlflow/Guards.qll +++ b/shared/controlflow/codeql/controlflow/Guards.qll @@ -1280,21 +1280,21 @@ module Make< } } - signature predicate guardChecksSig(Guard g, Expr e, boolean branch); + signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv); bindingset[this] signature class StateSig; private module WithState { - signature predicate guardChecksSig(Guard g, Expr e, boolean branch, State state); + signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv, State state); } /** * Extends a `BarrierGuard` input predicate with wrapped invocations. */ module ValidationWrapper { - private predicate guardChecksWithState(Guard g, Expr e, boolean branch, Unit state) { - guardChecks0(g, e, branch) and exists(state) + private predicate guardChecksWithState(Guard g, Expr e, GuardValue gv, Unit state) { + guardChecks0(g, e, gv) and exists(state) } private module StatefulWrapper = ValidationWrapperWithState; @@ -1366,7 +1366,7 @@ module Make< * Holds if the guard `g` validates the expression `e` upon evaluating to `val`. */ private predicate guardChecks(Guard g, Expr e, GuardValue val, State state) { - guardChecks0(g, e, val.asBooleanValue(), state) + guardChecks0(g, e, val, state) or exists(NonOverridableMethodCall call, ParameterPosition ppos, ArgumentPosition apos | g = call and From 09058e48aa6c68858d9e9b70dbad5cb738a79111 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 9 Dec 2025 16:30:07 +0100 Subject: [PATCH 2/4] Guards: Rename -WithState to Parameterized-. --- .../code/cpp/ir/dataflow/internal/SsaImpl.qll | 4 +- .../controlflow/codeql/controlflow/Guards.qll | 39 +++++++++---------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll index 32ce89e26742..8dc3513b444b 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll @@ -1064,8 +1064,8 @@ module BarrierGuardWithIntParam { DataFlowIntegrationInput::Guard g, SsaImpl::Definition def, IRGuards::GuardValue val, int indirectionIndex ) { - IRGuards::Guards_v1::ValidationWrapperWithState::guardChecksDef(g, def, - val, indirectionIndex) + IRGuards::Guards_v1::ParameterizedValidationWrapper::guardChecksDef(g, + def, val, indirectionIndex) } Node getABarrierNode(int indirectionIndex) { diff --git a/shared/controlflow/codeql/controlflow/Guards.qll b/shared/controlflow/codeql/controlflow/Guards.qll index 0e5cf25fecbf..b313afcdb6bb 100644 --- a/shared/controlflow/codeql/controlflow/Guards.qll +++ b/shared/controlflow/codeql/controlflow/Guards.qll @@ -1283,36 +1283,35 @@ module Make< signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv); bindingset[this] - signature class StateSig; + signature class ParamSig; - private module WithState { - signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv, State state); + private module WithParam { + signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv, P par); } /** * Extends a `BarrierGuard` input predicate with wrapped invocations. */ module ValidationWrapper { - private predicate guardChecksWithState(Guard g, Expr e, GuardValue gv, Unit state) { - guardChecks0(g, e, gv) and exists(state) + private predicate guardChecksWithParam(Guard g, Expr e, GuardValue gv, Unit par) { + guardChecks0(g, e, gv) and exists(par) } - private module StatefulWrapper = ValidationWrapperWithState; + private module ParameterizedWrapper = + ParameterizedValidationWrapper; /** * Holds if the guard `g` validates the SSA definition `def` upon evaluating to `val`. */ predicate guardChecksDef(Guard g, SsaDefinition def, GuardValue val) { - StatefulWrapper::guardChecksDef(g, def, val, _) + ParameterizedWrapper::guardChecksDef(g, def, val, _) } } /** * Extends a `BarrierGuard` input predicate with wrapped invocations. */ - module ValidationWrapperWithState< - StateSig State, WithState::guardChecksSig/4 guardChecks0> - { + module ParameterizedValidationWrapper::guardChecksSig/4 guardChecks0> { private import WrapperGuard /** @@ -1321,12 +1320,12 @@ module Make< * parameter has been validated by the given guard. */ private predicate validReturnInValidationWrapper( - ReturnExpr ret, ParameterPosition ppos, GuardValue retval, State state + ReturnExpr ret, ParameterPosition ppos, GuardValue retval, P par ) { exists(NonOverridableMethod m, SsaParameterInit param, Guard guard, GuardValue val | m.getAReturnExpr() = ret and param.getParameter() = m.getParameter(ppos) and - guardChecksDef(guard, param, val, state) + guardChecksDef(guard, param, val, par) | guard.valueControls(ret.getBasicBlock(), val) and relevantReturnExprValue(m, ret, retval) @@ -1341,7 +1340,7 @@ module Make< * that the argument has been validated by the given guard. */ private NonOverridableMethod validationWrapper( - ParameterPosition ppos, GuardValue retval, State state + ParameterPosition ppos, GuardValue retval, P par ) { forex(ReturnExpr ret | result.getAReturnExpr() = ret and @@ -1350,12 +1349,12 @@ module Make< disjointValues(notRetval, retval) ) | - validReturnInValidationWrapper(ret, ppos, retval, state) + validReturnInValidationWrapper(ret, ppos, retval, par) ) or exists(SsaParameterInit param, BasicBlock bb, Guard guard, GuardValue val | param.getParameter() = result.getParameter(ppos) and - guardChecksDef(guard, param, val, state) and + guardChecksDef(guard, param, val, par) and guard.valueControls(bb, val) and normalExitBlock(bb) and retval = TException(false) @@ -1365,12 +1364,12 @@ module Make< /** * Holds if the guard `g` validates the expression `e` upon evaluating to `val`. */ - private predicate guardChecks(Guard g, Expr e, GuardValue val, State state) { - guardChecks0(g, e, val, state) + private predicate guardChecks(Guard g, Expr e, GuardValue val, P par) { + guardChecks0(g, e, val, par) or exists(NonOverridableMethodCall call, ParameterPosition ppos, ArgumentPosition apos | g = call and - call.getMethod() = validationWrapper(ppos, val, state) and + call.getMethod() = validationWrapper(ppos, val, par) and call.getArgument(apos) = e and parameterMatch(pragma[only_bind_out](ppos), pragma[only_bind_out](apos)) ) @@ -1379,9 +1378,9 @@ module Make< /** * Holds if the guard `g` validates the SSA definition `def` upon evaluating to `val`. */ - predicate guardChecksDef(Guard g, SsaDefinition def, GuardValue val, State state) { + predicate guardChecksDef(Guard g, SsaDefinition def, GuardValue val, P par) { exists(Expr e | - guardChecks(g, e, val, state) and + guardChecks(g, e, val, par) and guardReadsSsaVar(e, def) ) } From 9cd2247b91d4fe55ca32283ff1e3dc1c3f44fed3 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 10 Dec 2025 11:21:01 +0100 Subject: [PATCH 3/4] Java: expose support for more general BarrierGuards. --- .../java/dataflow/internal/DataFlowUtil.qll | 30 +++++++++++++++++-- .../code/java/dataflow/internal/SsaImpl.qll | 4 +-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 00e7d15ee8b8..d96834a55331 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -375,13 +375,13 @@ class ContentSet instanceof Content { } /** - * Holds if the guard `g` validates the expression `e` upon evaluating to `branch`. + * Holds if the guard `g` validates the expression `e` upon evaluating to `gv`. * * The expression `e` is expected to be a syntactic part of the guard `g`. * For example, the guard `g` might be a call `isSafe(x)` and the expression `e` * the argument `x`. */ -signature predicate guardChecksSig(Guard g, Expr e, boolean branch); +signature predicate valueGuardChecksSig(Guard g, Expr e, GuardValue gv); /** * Provides a set of barrier nodes for a guard that validates an expression. @@ -389,10 +389,34 @@ signature predicate guardChecksSig(Guard g, Expr e, boolean branch); * This is expected to be used in `isBarrier`/`isSanitizer` definitions * in data flow and taint tracking. */ -module BarrierGuard { +module BarrierGuardValue { /** Gets a node that is safely guarded by the given guard check. */ Node getABarrierNode() { SsaFlow::asNode(result) = SsaImpl::DataFlowIntegration::BarrierGuard::getABarrierNode() } } + +/** + * Holds if the guard `g` validates the expression `e` upon evaluating to `branch`. + * + * The expression `e` is expected to be a syntactic part of the guard `g`. + * For example, the guard `g` might be a call `isSafe(x)` and the expression `e` + * the argument `x`. + */ +signature predicate guardChecksSig(Guard g, Expr e, boolean branch); + +/** + * Provides a set of barrier nodes for a guard that validates an expression. + * + * This is expected to be used in `isBarrier`/`isSanitizer` definitions + * in data flow and taint tracking. + */ +module BarrierGuard { + private predicate guardChecks0(Guard g, Expr e, GuardValue gv) { + guardChecks(g, e, gv.asBooleanValue()) + } + + /** Gets a node that is safely guarded by the given guard check. */ + Node getABarrierNode() { result = BarrierGuardValue::getABarrierNode() } +} diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 079fad797d8f..323f14f550ff 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -564,14 +564,14 @@ private module Cached { DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo) } - signature predicate guardChecksSig(Guards::Guard g, Expr e, boolean branch); + signature predicate guardChecksSig(Guards::Guard g, Expr e, Guards::GuardValue gv); cached // nothing is actually cached module BarrierGuard { private predicate guardChecksAdjTypes( Guards::Guards_v3::Guard g, Expr e, Guards::GuardValue gv ) { - guardChecks(g, e, gv.asBooleanValue()) + guardChecks(g, e, gv) } private predicate guardChecksWithWrappers( From eaa96864f7581dc3f1d93562da13ce014420fd7a Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 10 Dec 2025 12:20:56 +0100 Subject: [PATCH 4/4] Java: Extend test to cover assertion-like barrier guards. --- .../ql/test/library-tests/dataflow/ssa/A.java | 27 ++++++++++++++++++- .../test/library-tests/dataflow/ssa/test.ql | 10 +++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/java/ql/test/library-tests/dataflow/ssa/A.java b/java/ql/test/library-tests/dataflow/ssa/A.java index 79303697d646..d7047bec276a 100644 --- a/java/ql/test/library-tests/dataflow/ssa/A.java +++ b/java/ql/test/library-tests/dataflow/ssa/A.java @@ -4,7 +4,13 @@ void sink(Object o) { } boolean isSafe(Object o) { return o == null; } - void foo() { + void assertSafe(Object o) { if (o != null) throw new RuntimeException(); } + + private boolean wrapIsSafe(Object o) { return isSafe(o); } + + private void wrapAssertSafe(Object o) { assertSafe(o); } + + void test1() { Object x = source(); if (!isSafe(x)) { x = null; @@ -21,4 +27,23 @@ void foo() { } sink(x); } + + void test2() { + Object x = source(); + assertSafe(x); + sink(x); + } + + void test3() { + Object x = source(); + if (wrapIsSafe(x)) { + sink(x); + } + } + + void test4() { + Object x = source(); + wrapAssertSafe(x); + sink(x); + } } diff --git a/java/ql/test/library-tests/dataflow/ssa/test.ql b/java/ql/test/library-tests/dataflow/ssa/test.ql index cb601dab5eb1..0bea0aa71a1f 100644 --- a/java/ql/test/library-tests/dataflow/ssa/test.ql +++ b/java/ql/test/library-tests/dataflow/ssa/test.ql @@ -10,6 +10,14 @@ private predicate isSafe(Guard g, Expr checked, boolean branch) { ) } +private predicate assertSafe(Guard g, Expr checked, GuardValue gv) { + exists(MethodCall mc | g = mc | + mc.getMethod().hasName("assertSafe") and + checked = mc.getAnArgument() and + gv.getDualValue().isThrowsException() + ) +} + module TestConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr().(MethodCall).getMethod().hasName("source") @@ -21,6 +29,8 @@ module TestConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node = DataFlow::BarrierGuard::getABarrierNode() + or + node = DataFlow::BarrierGuardValue::getABarrierNode() } }