Skip to content

Query evaluation performance: UI5BindingPath.getNode() bottleneck and other optimization opportunities #349

@data-douser

Description

@data-douser

Summary

During a query evaluation profiling session using ql-mcp profile_codeql_query_from_logs, several performance bottlenecks were identified in the custom CodeQL queries across the UI5, CAP, and XSJS query packs. This issue tracks the evaluation-time (post-compilation) optimization opportunities.

Profiling context: 3 CodeQL JavaScript databases of varying sizes (5.8 MB and 11.5 MB stringpools), CodeQL CLI v2.25.1. The larger database is a real-world SAP UI5 monorepo with many packages.


Critical: UI5Xss.qlUI5BindingPath.getNode() super-linear scaling

File: javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll (line ~148)

The Problem

When UI5Xss.ql is evaluated standalone (via query_run) on the larger database (11.5 MB stringpool), it takes 33 minutes 28 seconds and produces 0 results. The same query on a smaller database (5.8 MB stringpool) takes only 8.4 seconds (also 0 results). That is a ~240x slowdown for ~2x the data size.

Profiling reveals that two dispatched variants of UI5BindingPath.getNode/0 account for 98.4% of the total 2,008-second evaluation time:

Predicate Duration Result Rows Strategy Dependencies
UI5BindingPath.getNode/0#dispred#...#fb 993s (49.5%) 1 SIMPLE_INTENSIONAL 29
UI5BindingPath.getNode/0#dispred#... 981s (48.9%) 1 SIMPLE_INTENSIONAL 28

Both predicates compute only 1 result row but take ~16.5 minutes each. On the smaller database, getNode doesn't even appear in the top 20 predicates.

Root Cause Analysis

The getNode() predicate in UI5View.qll has 4 disjuncts that join binding paths against models:

  1. Disjunct 1-1: Joins UI5BindingPath.getAbsolutePath() against JsonModel.getPathString(p) — requires enumerating all binding paths × all model path strings
  2. Disjunct 1-2: Joins UI5BindingPath.getPath() against JsonModel.getPathStringPropName() — similar cross-product
  3. Disjunct 1-3: Calls getNonStaticJsonModelNode() (already has pragma[nomagic])
  4. Disjunct 2: Calls getExternalModelNode() (already has pragma[nomagic])

The larger database has many more UI5 binding paths across many packages. The cross-product in disjuncts 1-1 and 1-2, combined with the inSameWebApp filtering applied late, appears to scale super-linearly with the number of binding paths.

Potential Optimizations

  1. Add pragma[nomagic] to disjuncts 1-1 and 1-2 to prevent inlining and cross-product explosion (disjuncts 3 and 4 already have this treatment)
  2. Add binding restrictions: Pre-filter binding paths by file/webapp scope before joining against model path strings
  3. Consider pragma[assume_small_delta] if the recursive descent in getAbsolutePath() is causing excessive intermediate results
  4. Evaluate whether early termination is possible: The query produces 0 results on both databases, yet the full binding path × model enumeration is still performed

Mitigating Factor

When run as part of the full UI5 query pack (via database_analyze), UI5Xss.ql benefits from shared predicate evaluation and takes only 0.5–2.5s. However, standalone execution (as happens during iterative development or single-query CI) remains problematic on larger databases.


Moderate: constructPathStringInner recursive string construction

File: javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll (line ~1093)

The Problem

constructPathStringInner/2 was the #3 hottest predicate when profiling UI5Xss on the large database (4.4s, 28,154 result rows, RECURSIVE_INTENSIONAL with 10 dependencies, 8 pipelines). On the smaller database, it was the #1 hottest predicate (539ms, 7,698 rows).

This recursive predicate walks object literal trees to build path strings like /p1/p2/p3. With deep or wide object structures, the number of intermediate path strings grows combinatorially.

Potential Optimization

Consider adding a depth bound to the recursion, or using pragma[assume_small_delta] if the object literal nesting depth is bounded in practice.


Moderate: TVarRefinementContext shared across multiple taint queries

The Problem

The type inference predicate TVarRefinementContext appears as the hottest predicate for multiple queries across packs:

Query Pack Duration Result Rows
UI5PathInjection.ql UI5 1.1–1.7s 10,381–27,449
ListLogInjectionSinks.ql UI5 0.9–1.7s 10,381–27,449
XSJSZipSlip.ql XSJS 1.3s 27,449
SensitiveExposureHeuristicSource.ql CAP 1.3s 27,449

This is a codeql/javascript-all library predicate and not directly modifiable here. However, query designs that depend heavily on type inference refinement contexts will inherit this baseline cost. Consider whether the custom queries can be structured to reduce dependence on full type inference where possible.


Lower Priority: CAP UnnecessarilyGrantedPrivilegedAccessRights.ql

File: javascript/frameworks/cap/src/bad-authn-authz/UnnecessarilyGrantedPrivilegedAccessRights/UnnecessarilyGrantedPrivilegedAccessRights.ql

The Problem

At 4.0s evaluation with 204 predicates, the hottest predicate is ControlFlowNode.getAPredecessor/0 at 1.6s producing 4,066,472 rows. This is a large intermediate result set from CFG traversal.

Potential Optimization

Investigate whether the query can restrict CFG traversal to relevant scopes earlier (e.g., only within CAP handler functions) rather than traversing the full program CFG.


Evaluation Performance Summary

Custom Query Packs (all eval-only, post-compilation)

Pack Queries Per-query Eval Range Slowest Query Notes
UI5 10 8–11s each UI5Xss (33m standalone on large DB) getNode() bottleneck
CAP 9 1.6–9.9s each SensitiveExposureHeuristicSource (9.9s) Reasonable
XSJS 7 2.8–6.6s each XSJSZipSlip (6.6s) Reasonable

Reference: Standard codeql/javascript-queries

Metric Value
Queries 202
Total eval time 40s
Mean 0.2s
Max 1.5s (PolynomialReDoS)
Queries > 10s 0

Checklist

  • Critical: Optimize UI5BindingPath.getNode() in UI5View.qll — add pragma[nomagic] to disjuncts 1-1/1-2, investigate binding path pre-filtering
  • Moderate: Review constructPathStringInner recursion in UI5.qll for depth-bounding opportunities
  • Moderate: Assess whether custom queries can reduce TVarRefinementContext dependence
  • Lower priority: Restrict CFG traversal scope in UnnecessarilyGrantedPrivilegedAccessRights.ql
  • Establish a performance regression test for standalone UI5Xss.ql evaluation time on a large UI5 monorepo database

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions