Skip to content

Commit 548f028

Browse files
esbenaStephan Brandauer
esbena
authored and
Stephan Brandauer
committed
address review comments
1 parent 57041aa commit 548f028

8 files changed

Lines changed: 93 additions & 122 deletions

File tree

‎javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointFeatures.qll‎

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,7 @@ private string getTokenFeature(DataFlow::Node endpoint, string featureName) {
1717
// Performance optimization: Restrict feature extraction to endpoints we've explicitly asked to featurize.
1818
endpoint = any(FeaturizationConfig cfg).getAnEndpointToFeaturize() and
1919
exists(EndpointFeature f | f.getName() = featureName and result = f.getValue(endpoint)) and
20-
isVettedFeature(featureName)
21-
}
22-
23-
predicate isVettedFeature(string featureName) {
24-
// allowlist of vetted features that are permitted in production
25-
featureName =
26-
any(EndpointFeature f |
27-
f instanceof EnclosingFunctionName or
28-
f instanceof CalleeName or
29-
f instanceof ReceiverName or
30-
f instanceof ArgumentIndex or
31-
f instanceof CalleeApiName or
32-
f instanceof CalleeAccessPath or
33-
f instanceof CalleeAccessPathWithStructuralInfo or
34-
f instanceof EnclosingFunctionBody
35-
).getName()
20+
featureName = getASupportedFeatureName()
3621
}
3722

3823
/**
@@ -206,7 +191,20 @@ private module FunctionNames {
206191
}
207192

208193
/** Get a name of a supported generic token-based feature. */
209-
string getASupportedFeatureName() { isVettedFeature(result) }
194+
string getASupportedFeatureName() {
195+
// allowlist of vetted features that are permitted in production
196+
result =
197+
any(EndpointFeature f |
198+
f instanceof EnclosingFunctionName or
199+
f instanceof CalleeName or
200+
f instanceof ReceiverName or
201+
f instanceof ArgumentIndex or
202+
f instanceof CalleeApiName or
203+
f instanceof CalleeAccessPath or
204+
f instanceof CalleeAccessPathWithStructuralInfo or
205+
f instanceof EnclosingFunctionBody
206+
).getName()
207+
}
210208

211209
/**
212210
* Generic token-based features for ATM.
@@ -253,7 +251,7 @@ abstract class EndpointFeature extends TEndpointFeature {
253251
*/
254252
abstract string getValue(DataFlow::Node endpoint);
255253

256-
string toString() { result = getName() }
254+
string toString() { result = this.getName() }
257255
}
258256

259257
/**
@@ -456,7 +454,7 @@ private module SyntacticUtilities {
456454
w.getRhs() = node and
457455
result = getSimpleParameterAccessPath(w.getBase()) + "." + getPropertyNameOrUnknown(w)
458456
)
459-
else result = "?"
457+
else result = getUnknownSymbol()
460458
}
461459

462460
/**
@@ -497,10 +495,12 @@ private module SyntacticUtilities {
497495
if node instanceof DataFlow::InvokeNode
498496
then
499497
result = getSimpleAccessPath(node.(DataFlow::InvokeNode).getCalleeNode()) + "()"
500-
else result = "?"
498+
else result = getUnknownSymbol()
501499
)
502500
}
503501

502+
string getUnknownSymbol() { result = "?" }
503+
504504
/**
505505
* Gets the imported path.
506506
*
@@ -514,15 +514,17 @@ private module SyntacticUtilities {
514514
exists(string p | p = i.getImportedPath().getValue() |
515515
if p.matches(".%") then result = "\"p\"" else result = "!" // hide absolute imports from the ML training
516516
)
517-
else result = "?"
517+
else result = getUnknownSymbol()
518518
}
519-
}
520519

521-
/**
522-
* Gets the property name of a property reference or `?` if it is unknown.
523-
*/
524-
string getPropertyNameOrUnknown(DataFlow::PropRef ref) {
525-
if exists(ref.getPropertyName()) then result = ref.getPropertyName() else result = "?"
520+
/**
521+
* Gets the property name of a property reference or `?` if it is unknown.
522+
*/
523+
string getPropertyNameOrUnknown(DataFlow::PropRef ref) {
524+
if exists(ref.getPropertyName())
525+
then result = ref.getPropertyName()
526+
else result = getUnknownSymbol()
527+
}
526528
}
527529

528530
/**
@@ -537,11 +539,9 @@ class Callee_AccessPath extends EndpointFeature, TCallee_AccessPath {
537539

538540
override string getValue(DataFlow::Node endpoint) {
539541
exists(DataFlow::InvokeNode invk |
540-
exists(string path |
541-
path = SyntacticUtilities::getSimpleAccessPath(invk.getCalleeNode()) and
542-
// collapse the unknown path to the empty string, as is convention for old features
543-
if path = "?" then result = "" else result = path
544-
) and
542+
result = SyntacticUtilities::getSimpleAccessPath(invk.getCalleeNode()) and
543+
// ignore the unknown path
544+
not result = SyntacticUtilities::getUnknownSymbol() and
545545
(
546546
invk.getAnArgument() = endpoint or
547547
SyntacticUtilities::getANestedInitializerValue(invk.getAnArgument()
@@ -586,20 +586,14 @@ class Input_ArgumentIndexAndAccessPathFromCallee extends EndpointFeature,
586586
class Input_AccessPathFromCallee extends EndpointFeature, TInput_AccessPathFromCallee {
587587
override string getName() { result = "Input_AccessPathFromCallee" }
588588

589-
private string getValueMaybe(DataFlow::Node endpoint) {
589+
override string getValue(DataFlow::Node endpoint) {
590590
exists(DataFlow::InvokeNode invk |
591591
result = SyntacticUtilities::getSimpleParameterAccessPath(endpoint) and
592592
SyntacticUtilities::getANestedInitializerValue(invk.getAnArgument()
593593
.asExpr()
594594
.getUnderlyingValue()).flow() = endpoint
595595
)
596596
}
597-
598-
override string getValue(DataFlow::Node endpoint) {
599-
if exists(this.getValueMaybe(endpoint))
600-
then result = this.getValueMaybe(endpoint)
601-
else result = ""
602-
}
603597
}
604598

605599
/**

‎javascript/ql/experimental/adaptivethreatmodeling/test/generic_feature_testing/EmptyFeature.expected‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +0,0 @@
1-
| calleeApiName |
2-
| enclosingFunctionBody |
3-
| enclosingFunctionName |

‎javascript/ql/experimental/adaptivethreatmodeling/test/generic_feature_testing/EmptyFeature.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import javascript
22
import experimental.adaptivethreatmodeling.EndpointFeatures
3+
import experimental.adaptivethreatmodeling.FeaturizationConfig
34
import TestUtil
45

56
// every feature must produce a value for at least one endpoint, otherwise the feature is completely broken, or a relevant test example is missing

‎javascript/ql/experimental/adaptivethreatmodeling/test/generic_feature_testing/FeatureValue.expected‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,127 @@
11
| test.html:2:61:2:68 | endpoint | Callee_AccessPath | $event.target.files.item |
2-
| test.html:2:61:2:68 | endpoint | Input_AccessPathFromCallee | |
32
| test.html:2:61:2:68 | endpoint | Input_ArgumentIndex | 0 |
43
| test.html:2:61:2:68 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
54
| test.html:2:61:2:68 | endpoint | argumentIndex | 0 |
65
| test.html:2:61:2:68 | endpoint | calleeAccessPath | |
76
| test.html:2:61:2:68 | endpoint | calleeAccessPathWithStructuralInfo | |
87
| test.html:2:61:2:68 | endpoint | calleeName | item |
98
| test.js:2:7:2:14 | endpoint | Callee_AccessPath | f |
10-
| test.js:2:7:2:14 | endpoint | Input_AccessPathFromCallee | |
119
| test.js:2:7:2:14 | endpoint | Input_ArgumentIndex | 0 |
1210
| test.js:2:7:2:14 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
1311
| test.js:2:7:2:14 | endpoint | argumentIndex | 0 |
1412
| test.js:2:7:2:14 | endpoint | calleeAccessPath | |
1513
| test.js:2:7:2:14 | endpoint | calleeAccessPathWithStructuralInfo | |
1614
| test.js:2:7:2:14 | endpoint | calleeName | f |
15+
| test.js:2:7:2:14 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
16+
| test.js:2:7:2:14 | endpoint | enclosingFunctionName | |
1717
| test.js:3:11:3:18 | endpoint | Callee_AccessPath | f |
1818
| test.js:3:11:3:18 | endpoint | Input_AccessPathFromCallee | 0.p |
1919
| test.js:3:11:3:18 | endpoint | Input_ArgumentIndex | 0 |
2020
| test.js:3:11:3:18 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0.p |
2121
| test.js:3:11:3:18 | endpoint | calleeAccessPath | |
2222
| test.js:3:11:3:18 | endpoint | calleeAccessPathWithStructuralInfo | |
23+
| test.js:3:11:3:18 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
24+
| test.js:3:11:3:18 | endpoint | enclosingFunctionName | |
2325
| test.js:4:15:4:22 | endpoint | Callee_AccessPath | f |
2426
| test.js:4:15:4:22 | endpoint | Input_AccessPathFromCallee | 0.p.q |
2527
| test.js:4:15:4:22 | endpoint | Input_ArgumentIndex | 0 |
2628
| test.js:4:15:4:22 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0.p.q |
2729
| test.js:4:15:4:22 | endpoint | calleeAccessPath | |
2830
| test.js:4:15:4:22 | endpoint | calleeAccessPathWithStructuralInfo | |
31+
| test.js:4:15:4:22 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
32+
| test.js:4:15:4:22 | endpoint | enclosingFunctionName | |
2933
| test.js:5:9:5:16 | endpoint | Callee_AccessPath | o.m |
30-
| test.js:5:9:5:16 | endpoint | Input_AccessPathFromCallee | |
3134
| test.js:5:9:5:16 | endpoint | Input_ArgumentIndex | 0 |
3235
| test.js:5:9:5:16 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
3336
| test.js:5:9:5:16 | endpoint | argumentIndex | 0 |
3437
| test.js:5:9:5:16 | endpoint | calleeAccessPath | |
3538
| test.js:5:9:5:16 | endpoint | calleeAccessPathWithStructuralInfo | |
3639
| test.js:5:9:5:16 | endpoint | calleeName | m |
40+
| test.js:5:9:5:16 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
41+
| test.js:5:9:5:16 | endpoint | enclosingFunctionName | |
3742
| test.js:5:9:5:16 | endpoint | receiverName | o |
3843
| test.js:6:13:6:20 | endpoint | Callee_AccessPath | o.m |
3944
| test.js:6:13:6:20 | endpoint | Input_AccessPathFromCallee | 0.p |
4045
| test.js:6:13:6:20 | endpoint | Input_ArgumentIndex | 0 |
4146
| test.js:6:13:6:20 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0.p |
4247
| test.js:6:13:6:20 | endpoint | calleeAccessPath | |
4348
| test.js:6:13:6:20 | endpoint | calleeAccessPathWithStructuralInfo | |
49+
| test.js:6:13:6:20 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
50+
| test.js:6:13:6:20 | endpoint | enclosingFunctionName | |
4451
| test.js:7:17:7:24 | endpoint | Callee_AccessPath | o.m |
4552
| test.js:7:17:7:24 | endpoint | Input_AccessPathFromCallee | 0.p.q |
4653
| test.js:7:17:7:24 | endpoint | Input_ArgumentIndex | 0 |
4754
| test.js:7:17:7:24 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0.p.q |
4855
| test.js:7:17:7:24 | endpoint | calleeAccessPath | |
4956
| test.js:7:17:7:24 | endpoint | calleeAccessPathWithStructuralInfo | |
57+
| test.js:7:17:7:24 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
58+
| test.js:7:17:7:24 | endpoint | enclosingFunctionName | |
5059
| test.js:8:11:8:18 | endpoint | Callee_AccessPath | F |
51-
| test.js:8:11:8:18 | endpoint | Input_AccessPathFromCallee | |
5260
| test.js:8:11:8:18 | endpoint | Input_ArgumentIndex | 0 |
5361
| test.js:8:11:8:18 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | ? |
5462
| test.js:8:11:8:18 | endpoint | calleeAccessPath | |
5563
| test.js:8:11:8:18 | endpoint | calleeAccessPathWithStructuralInfo | |
64+
| test.js:8:11:8:18 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
65+
| test.js:8:11:8:18 | endpoint | enclosingFunctionName | |
5666
| test.js:9:17:9:24 | endpoint | Callee_AccessPath | o.m().m().m |
57-
| test.js:9:17:9:24 | endpoint | Input_AccessPathFromCallee | |
5867
| test.js:9:17:9:24 | endpoint | Input_ArgumentIndex | 0 |
5968
| test.js:9:17:9:24 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
6069
| test.js:9:17:9:24 | endpoint | argumentIndex | 0 |
6170
| test.js:9:17:9:24 | endpoint | calleeAccessPath | |
6271
| test.js:9:17:9:24 | endpoint | calleeAccessPathWithStructuralInfo | |
6372
| test.js:9:17:9:24 | endpoint | calleeName | m |
73+
| test.js:9:17:9:24 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
74+
| test.js:9:17:9:24 | endpoint | enclosingFunctionName | |
6475
| test.js:10:9:10:16 | endpoint | Callee_AccessPath | f() |
65-
| test.js:10:9:10:16 | endpoint | Input_AccessPathFromCallee | |
6676
| test.js:10:9:10:16 | endpoint | Input_ArgumentIndex | 0 |
6777
| test.js:10:9:10:16 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
6878
| test.js:10:9:10:16 | endpoint | argumentIndex | 0 |
6979
| test.js:10:9:10:16 | endpoint | calleeAccessPath | |
7080
| test.js:10:9:10:16 | endpoint | calleeAccessPathWithStructuralInfo | |
81+
| test.js:10:9:10:16 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
82+
| test.js:10:9:10:16 | endpoint | enclosingFunctionName | |
7183
| test.js:11:12:11:19 | endpoint | Callee_AccessPath | o.?.m |
72-
| test.js:11:12:11:19 | endpoint | Input_AccessPathFromCallee | |
7384
| test.js:11:12:11:19 | endpoint | Input_ArgumentIndex | 0 |
7485
| test.js:11:12:11:19 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
7586
| test.js:11:12:11:19 | endpoint | argumentIndex | 0 |
7687
| test.js:11:12:11:19 | endpoint | calleeAccessPath | |
7788
| test.js:11:12:11:19 | endpoint | calleeAccessPathWithStructuralInfo | |
7889
| test.js:11:12:11:19 | endpoint | calleeName | m |
90+
| test.js:11:12:11:19 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
91+
| test.js:11:12:11:19 | endpoint | enclosingFunctionName | |
7992
| test.js:12:16:12:23 | endpoint | Callee_AccessPath | o.m.?.p.m |
80-
| test.js:12:16:12:23 | endpoint | Input_AccessPathFromCallee | |
8193
| test.js:12:16:12:23 | endpoint | Input_ArgumentIndex | 0 |
8294
| test.js:12:16:12:23 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
8395
| test.js:12:16:12:23 | endpoint | argumentIndex | 0 |
8496
| test.js:12:16:12:23 | endpoint | calleeAccessPath | |
8597
| test.js:12:16:12:23 | endpoint | calleeAccessPathWithStructuralInfo | |
8698
| test.js:12:16:12:23 | endpoint | calleeName | m |
99+
| test.js:12:16:12:23 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
100+
| test.js:12:16:12:23 | endpoint | enclosingFunctionName | |
87101
| test.js:13:15:13:22 | endpoint | Callee_AccessPath | (await p) |
88-
| test.js:13:15:13:22 | endpoint | Input_AccessPathFromCallee | |
89102
| test.js:13:15:13:22 | endpoint | Input_ArgumentIndex | 0 |
90103
| test.js:13:15:13:22 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
91104
| test.js:13:15:13:22 | endpoint | argumentIndex | 0 |
92105
| test.js:13:15:13:22 | endpoint | calleeAccessPath | |
93106
| test.js:13:15:13:22 | endpoint | calleeAccessPathWithStructuralInfo | |
107+
| test.js:13:15:13:22 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
108+
| test.js:13:15:13:22 | endpoint | enclosingFunctionName | |
109+
| test.js:14:27:14:34 | endpoint | Callee_AccessPath | import(!).bar.baz |
110+
| test.js:14:27:14:34 | endpoint | Input_ArgumentIndex | 0 |
111+
| test.js:14:27:14:34 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
112+
| test.js:14:27:14:34 | endpoint | argumentIndex | 0 |
113+
| test.js:14:27:14:34 | endpoint | calleeAccessPath | foo bar baz |
114+
| test.js:14:27:14:34 | endpoint | calleeAccessPathWithStructuralInfo | foo member bar member baz instanceorreturn |
115+
| test.js:14:27:14:34 | endpoint | calleeApiName | foo |
116+
| test.js:14:27:14:34 | endpoint | calleeName | baz |
117+
| test.js:14:27:14:34 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
118+
| test.js:14:27:14:34 | endpoint | enclosingFunctionName | |
119+
| test.js:16:13:16:20 | endpoint | Callee_AccessPath | bar |
120+
| test.js:16:13:16:20 | endpoint | Input_ArgumentIndex | 0 |
121+
| test.js:16:13:16:20 | endpoint | Input_ArgumentIndexAndAccessPathFromCallee | 0 |
122+
| test.js:16:13:16:20 | endpoint | argumentIndex | 0 |
123+
| test.js:16:13:16:20 | endpoint | calleeAccessPath | |
124+
| test.js:16:13:16:20 | endpoint | calleeAccessPathWithStructuralInfo | |
125+
| test.js:16:13:16:20 | endpoint | calleeName | bar |
126+
| test.js:16:13:16:20 | endpoint | enclosingFunctionBody | f endpoint f p endpoint f p q endpoint o m endpoint o m p endpoint o m p q endpoint F endpoint o m m m endpoint f endpoint o x m endpoint o m x p m endpoint p endpoint foo bar baz endpoint foo bar endpoint |
127+
| test.js:16:13:16:20 | endpoint | enclosingFunctionName | |

0 commit comments

Comments
 (0)