Skip to content

Commit c9a8723

Browse files
committed
JS: Factor helper predicate to improve SensitiveWrite performance
1 parent 66c1629 commit c9a8723

File tree

1 file changed

+37
-11
lines changed

1 file changed

+37
-11
lines changed

javascript/ql/lib/semmle/javascript/security/SensitiveActions.qll

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,48 @@ abstract class SensitiveVariableAccess extends SensitiveExpr {
8181
/** A write to a location that might contain sensitive data. */
8282
abstract class SensitiveWrite extends DataFlow::Node { }
8383

84+
/**
85+
* Holds if `node` is a write to a variable or property named `name`.
86+
*
87+
* Helper predicate factored out for performance,
88+
* to filter `name` as much as possible before using it in
89+
* regex matching.
90+
*/
91+
pragma[nomagic]
92+
private predicate writesProperty(DataFlow::Node node, string name) {
93+
exists(DataFlow::PropWrite pwn |
94+
pwn.getPropertyName() = name and
95+
pwn.getRhs() = node
96+
)
97+
or
98+
exists(VarDef v | v.getAVariable().getName() = name |
99+
if exists(v.getSource())
100+
then v.getSource() = node.asExpr()
101+
else node = DataFlow::ssaDefinitionNode(SSA::definition(v))
102+
)
103+
}
104+
84105
/** A write to a variable or property that might contain sensitive data. */
85106
private class BasicSensitiveWrite extends SensitiveWrite {
86107
SensitiveDataClassification classification;
87108

88109
BasicSensitiveWrite() {
89-
exists(string name | nameIndicatesSensitiveData(name, classification) |
90-
exists(DataFlow::PropWrite pwn |
91-
pwn.getPropertyName() = name and
92-
pwn.getRhs() = this
93-
)
94-
or
95-
exists(VarDef v | v.getAVariable().getName() = name |
96-
if exists(v.getSource())
97-
then v.getSource() = this.asExpr()
98-
else this = DataFlow::ssaDefinitionNode(SSA::definition(v))
99-
)
110+
exists(string name |
111+
/*
112+
* PERFORMANCE OPTIMISATION:
113+
* `nameIndicatesSensitiveData` performs a `regexpMatch` on `name`.
114+
* To carry out a regex match, we must first compute the Cartesian product
115+
* of all possible `name`s and regexes, then match.
116+
* To keep this product as small as possible,
117+
* we want to filter `name` as much as possible before the product.
118+
*
119+
* Do this by factoring out a helper predicate containing the filtering
120+
* logic that restricts `name`. This helper predicate will get picked first
121+
* in the join order, since it is the only call here that binds `name`.
122+
*/
123+
124+
writesProperty(this, name) and
125+
nameIndicatesSensitiveData(name, classification)
100126
)
101127
}
102128

0 commit comments

Comments
 (0)