From e5ae7e023109fbd4ce8cc89762241bcf0287875b Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 10:48:17 +0100 Subject: [PATCH] JS: Fix bad join in isOptionallySanitizedEdgeInternal This was previously called from isBarrier(node, state) but without restricting the state. The call was therefore moved to isBarrier(node), but this caused some optimisation changes resulting in a bad join. --- .../security/dataflow/DomBasedXssCustomizations.qll | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 731ef1eb721c..b9f27c6a8c2e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -329,6 +329,12 @@ module DomBasedXss { */ deprecated predicate isOptionallySanitizedEdge = isOptionallySanitizedEdgeInternal/2; + bindingset[call] + pragma[inline_late] + private SsaVariable getSanitizedSsaVariable(HtmlSanitizerCall call) { + call.getAnArgument().asExpr().(VarAccess).getVariable() = result.getSourceVariable() + } + private predicate isOptionallySanitizedEdgeInternal(DataFlow::Node pred, DataFlow::Node succ) { exists(HtmlSanitizerCall sanitizer | // sanitized = sanitize ? sanitizer(source) : source; @@ -348,7 +354,7 @@ module DomBasedXss { count(phi.getAnInput()) = 2 and not a = b and sanitizer = DataFlow::valueNode(a.getDef().getSource()) and - sanitizer.getAnArgument().asExpr().(VarAccess).getVariable() = b.getSourceVariable() + getSanitizedSsaVariable(sanitizer) = b | pred = DataFlow::ssaDefinitionNode(b) and succ = DataFlow::ssaDefinitionNode(phi)