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.
github · Dec 16, 2024 · e5ae7e0 · e5ae7e0
1 parent 947b785
commit e5ae7e0
Showing 1 changed file with 7 additions and 1 deletion.
diff --git 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)