Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 103c8ed

Browse files
committedDec 22, 2021
JS: Improve performance of Xss::isOptionallySanitizedEdge
When join-ordering and evaluating this conjunction, it is preferable to start with the relatively small set of `sanitizer` calls, then compute the set of SSA variables accessed as the arguments of those sanitizer calls, then reason about how those variables are used in phi nodes. Use directional binding pragmas to encourage this join order by picking `sanitizer` first, and discourage picking the opposite join order starting with `phi`. This impacts performance of the ATM XSS queries on large databases like Node, where computing all variable accesses from phi nodes leads to 435M+ tuples.
1 parent d0d6506 commit 103c8ed

File tree

1 file changed

+21
-2
lines changed
  • javascript/ql/lib/semmle/javascript/security/dataflow

1 file changed

+21
-2
lines changed
 

‎javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,27 @@ module DomBasedXss {
437437
b = phi.getAnInput().getDefinition() and
438438
count(phi.getAnInput()) = 2 and
439439
not a = b and
440-
sanitizer = DataFlow::valueNode(a.getDef().getSource()) and
441-
sanitizer.getAnArgument().asExpr().(VarAccess).getVariable() = b.getSourceVariable()
440+
/*
441+
* Performance optimisation:
442+
*
443+
* When join-ordering and evaluating this conjunction,
444+
* it is preferable to start with the relatively small set of
445+
* `sanitizer` calls, then compute the set of SSA variables accessed
446+
* as the arguments of those sanitizer calls, then reason about how
447+
* those variables are used in phi nodes.
448+
*
449+
* Use directional binding pragmas to encourage this join order,
450+
* starting with `sanitizer`.
451+
*
452+
* Without these pragmas, the join orderer may choose the opposite order:
453+
* start with all `phi` nodes, then compute the set of SSA variables involved,
454+
* then the (potentially large) set of accesses to those variables,
455+
* then the set of accesses used as the argument of a sanitizer call.
456+
*/
457+
458+
pragma[only_bind_out](sanitizer) = DataFlow::valueNode(a.getDef().getSource()) and
459+
pragma[only_bind_out](sanitizer.getAnArgument().asExpr()) =
460+
b.getSourceVariable().getAnAccess()
442461
|
443462
pred = DataFlow::ssaDefinitionNode(b) and
444463
succ = DataFlow::ssaDefinitionNode(phi)

0 commit comments

Comments
 (0)
Please sign in to comment.