Skip to content

Commit f618d43

Browse files
esbenaesbena
committed
JS: simplify HTTP::ContainerCollection, and improve expressivity(!)
1 parent 44ebf84 commit f618d43

3 files changed

Lines changed: 43 additions & 26 deletions

File tree

‎javascript/ql/src/semmle/javascript/Collections.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ private class PseudoProperty extends TypeTrackingPseudoProperty {
3131
* `load`/`store`/`loadStore` can be used in the `CollectionsTypeTracking` module.
3232
* (Thereby avoiding naming conflicts with a "cousin" `AdditionalFlowStep` implementation.)
3333
*/
34-
abstract private class CollectionFlowStep extends DataFlow::AdditionalFlowStep {
34+
abstract class CollectionFlowStep extends DataFlow::AdditionalFlowStep {
3535
final override predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
3636

3737
final override predicate step(

‎javascript/ql/src/semmle/javascript/frameworks/HTTP.qll‎

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import javascript
66
private import semmle.javascript.DynamicPropertyAccess
7+
private import semmle.javascript.dataflow.internal.StepSummary
78

89
module HTTP {
910
/**
@@ -575,36 +576,32 @@ module HTTP {
575576
}
576577

577578
/**
578-
* A map that contains one or more route potential handlers.
579+
* A collection that contains one or more route potential handlers.
579580
*/
580-
private class ContainerMap extends Range {
581-
ContainerMap() {
582-
this = DataFlow::globalVarRef("Map").getAnInstantiation() and
583-
exists(RouteHandlerCandidate candidate |
584-
candidate.flowsTo(this.getAMethodCall("set").getArgument(1))
585-
)
586-
}
587-
588-
DataFlow::SourceNode trackRouteHandler(
589-
DataFlow::TypeTracker t, RouteHandlerCandidate candidate
590-
) {
591-
exists(DataFlow::MethodCallNode set, DataFlow::Node setKey |
592-
setKey = set.getArgument(0) and
593-
this.getAMethodCall("set") = set and
594-
candidate.flowsTo(set.getArgument(1)) and
595-
result = this and // start type tracking on the Map object, because the route-handler is contained inside the Map.
596-
t.startInProp(DataFlow::PseudoProperties::mapValue(setKey))
597-
)
598-
or
599-
exists(DataFlow::TypeTracker t2 | result = trackRouteHandler(t2, candidate).track(t2, t))
600-
or
601-
exists(DataFlow::TypeTracker t2 |
602-
result = CollectionsTypeTracking::collectionStep(trackRouteHandler(t2, candidate), t, t2)
581+
private class ContainerCollection extends HTTP::RouteHandlerCandidateContainer::Range {
582+
ContainerCollection() {
583+
this = DataFlow::globalVarRef("Map").getAnInstantiation() and // restrict to Map for now
584+
exists(
585+
CollectionFlowStep store, DataFlow::Node storeTo, DataFlow::Node input,
586+
RouteHandlerCandidate candidate
587+
|
588+
this.flowsTo(storeTo) and
589+
store.store(input, storeTo, _) and
590+
candidate.flowsTo(input)
603591
)
604592
}
605593

606594
override DataFlow::SourceNode getRouteHandler(DataFlow::SourceNode access) {
607-
access = trackRouteHandler(DataFlow::TypeTracker::end(), result)
595+
exists(
596+
DataFlow::Node input, TypeTrackingPseudoProperty key, CollectionFlowStep store,
597+
CollectionFlowStep load, DataFlow::Node storeTo, DataFlow::Node loadFrom
598+
|
599+
this.flowsTo(storeTo) and
600+
store.store(input, storeTo, key) and
601+
result.(RouteHandlerCandidate).flowsTo(input) and
602+
ref(this).flowsTo(loadFrom) and
603+
load.load(loadFrom, access, key)
604+
)
608605
}
609606
}
610607
}

‎javascript/ql/test/library-tests/frameworks/Express/tests.expected‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,8 @@ test_isRequest
796796
| src/advanced-routehandler-registration.js:124:46:124:48 | req |
797797
| src/advanced-routehandler-registration.js:156:22:156:24 | req |
798798
| src/advanced-routehandler-registration.js:156:47:156:49 | req |
799+
| src/advanced-routehandler-registration.js:157:28:157:30 | req |
800+
| src/advanced-routehandler-registration.js:157:53:157:55 | req |
799801
| src/controllers/handler-in-bulk-require.js:1:45:1:47 | req |
800802
| src/csurf-example.js:20:28:20:30 | req |
801803
| src/csurf-example.js:22:35:22:37 | req |
@@ -1046,6 +1048,7 @@ test_ResponseExpr
10461048
| src/advanced-routehandler-registration.js:123:26:123:28 | res | src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) |
10471049
| src/advanced-routehandler-registration.js:124:26:124:28 | res | src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) |
10481050
| src/advanced-routehandler-registration.js:156:27:156:29 | res | src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) |
1051+
| src/advanced-routehandler-registration.js:157:33:157:35 | res | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
10491052
| src/controllers/handler-in-bulk-require.js:1:50:1:52 | res | src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined |
10501053
| src/csurf-example.js:20:33:20:35 | res | src/csurf-example.js:20:18:23:1 | functio ... () })\\n} |
10511054
| src/csurf-example.js:22:3:22:5 | res | src/csurf-example.js:20:18:23:1 | functio ... () })\\n} |
@@ -1549,6 +1552,7 @@ test_RouteHandler_getAResponseExpr
15491552
| src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:123:26:123:28 | res |
15501553
| src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:124:26:124:28 | res |
15511554
| src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) | src/advanced-routehandler-registration.js:156:27:156:29 | res |
1555+
| src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:157:33:157:35 | res |
15521556
| src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined | src/controllers/handler-in-bulk-require.js:1:50:1:52 | res |
15531557
| src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:20:33:20:35 | res |
15541558
| src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:22:3:22:5 | res |
@@ -1687,6 +1691,7 @@ test_isResponse
16871691
| src/advanced-routehandler-registration.js:123:26:123:28 | res |
16881692
| src/advanced-routehandler-registration.js:124:26:124:28 | res |
16891693
| src/advanced-routehandler-registration.js:156:27:156:29 | res |
1694+
| src/advanced-routehandler-registration.js:157:33:157:35 | res |
16901695
| src/controllers/handler-in-bulk-require.js:1:50:1:52 | res |
16911696
| src/csurf-example.js:20:33:20:35 | res |
16921697
| src/csurf-example.js:22:3:22:5 | res |
@@ -1835,6 +1840,8 @@ test_RouteSetup_getARouteHandler
18351840
| src/advanced-routehandler-registration.js:118:1:118:30 | app.get ... utes.a) | src/route-collection.js:2:6:2:35 | (req, r ... og(req) |
18361841
| src/advanced-routehandler-registration.js:119:1:119:30 | app.get ... utes.b) | src/advanced-routehandler-registration.js:119:14:119:29 | importedRoutes.b |
18371842
| src/advanced-routehandler-registration.js:119:1:119:30 | app.get ... utes.b) | src/route-collection.js:3:6:3:35 | (req, r ... og(req) |
1843+
| src/advanced-routehandler-registration.js:125:29:125:41 | app.get(k, v) | src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) |
1844+
| src/advanced-routehandler-registration.js:125:29:125:41 | app.get(k, v) | src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) |
18381845
| src/advanced-routehandler-registration.js:125:29:125:41 | app.get(k, v) | src/advanced-routehandler-registration.js:125:20:125:20 | v |
18391846
| src/advanced-routehandler-registration.js:125:29:125:41 | app.get(k, v) | src/advanced-routehandler-registration.js:125:23:125:23 | k |
18401847
| src/advanced-routehandler-registration.js:126:1:126:32 | app.get ... t("a")) | src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) |
@@ -1851,9 +1858,12 @@ test_RouteSetup_getARouteHandler
18511858
| src/advanced-routehandler-registration.js:150:2:150:14 | app.get(k, v) | src/advanced-routehandler-registration.js:150:13:150:13 | v |
18521859
| src/advanced-routehandler-registration.js:153:1:153:41 | app.get ... KEY!")) | src/advanced-routehandler-registration.js:153:14:153:40 | routesM ... _KEY!") |
18531860
| src/advanced-routehandler-registration.js:160:1:160:33 | app.get ... t("c")) | src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) |
1861+
| src/advanced-routehandler-registration.js:160:1:160:33 | app.get ... t("c")) | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
18541862
| src/advanced-routehandler-registration.js:160:1:160:33 | app.get ... t("c")) | src/advanced-routehandler-registration.js:160:14:160:32 | routesMap2.get("c") |
1863+
| src/advanced-routehandler-registration.js:161:1:161:39 | app.get ... own())) | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
18551864
| src/advanced-routehandler-registration.js:161:1:161:39 | app.get ... own())) | src/advanced-routehandler-registration.js:161:14:161:38 | routesM ... nown()) |
18561865
| src/advanced-routehandler-registration.js:162:1:162:23 | app.get ... nown()) | src/advanced-routehandler-registration.js:162:14:162:22 | unknown() |
1866+
| src/advanced-routehandler-registration.js:163:1:163:33 | app.get ... t("f")) | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
18571867
| src/advanced-routehandler-registration.js:163:1:163:33 | app.get ... t("f")) | src/advanced-routehandler-registration.js:163:14:163:32 | routesMap2.get("f") |
18581868
| src/auth.js:4:1:4:53 | app.use ... d' }})) | src/auth.js:4:9:4:52 | basicAu ... rd' }}) |
18591869
| src/csurf-example.js:13:1:13:20 | app.use('/api', api) | src/csurf-example.js:10:11:10:27 | createApiRouter() |
@@ -2233,6 +2243,7 @@ test_RouteHandler
22332243
| src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:123:21:123:23 | req | src/advanced-routehandler-registration.js:123:26:123:28 | res |
22342244
| src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:124:21:124:23 | req | src/advanced-routehandler-registration.js:124:26:124:28 | res |
22352245
| src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) | src/advanced-routehandler-registration.js:156:22:156:24 | req | src/advanced-routehandler-registration.js:156:27:156:29 | res |
2246+
| src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:157:28:157:30 | req | src/advanced-routehandler-registration.js:157:33:157:35 | res |
22362247
| src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined | src/controllers/handler-in-bulk-require.js:1:45:1:47 | req | src/controllers/handler-in-bulk-require.js:1:50:1:52 | res |
22372248
| src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:20:28:20:30 | req | src/csurf-example.js:20:33:20:35 | res |
22382249
| src/csurf-example.js:25:22:27:1 | functio ... ere')\\n} | src/csurf-example.js:25:32:25:34 | req | src/csurf-example.js:25:37:25:39 | res |
@@ -2415,6 +2426,8 @@ test_RequestExpr
24152426
| src/advanced-routehandler-registration.js:124:46:124:48 | req | src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) |
24162427
| src/advanced-routehandler-registration.js:156:22:156:24 | req | src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) |
24172428
| src/advanced-routehandler-registration.js:156:47:156:49 | req | src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) |
2429+
| src/advanced-routehandler-registration.js:157:28:157:30 | req | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
2430+
| src/advanced-routehandler-registration.js:157:53:157:55 | req | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) |
24182431
| src/controllers/handler-in-bulk-require.js:1:45:1:47 | req | src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined |
24192432
| src/csurf-example.js:20:28:20:30 | req | src/csurf-example.js:20:18:23:1 | functio ... () })\\n} |
24202433
| src/csurf-example.js:22:35:22:37 | req | src/csurf-example.js:20:18:23:1 | functio ... () })\\n} |
@@ -2525,6 +2538,8 @@ test_RouteHandler_getARequestExpr
25252538
| src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:124:46:124:48 | req |
25262539
| src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) | src/advanced-routehandler-registration.js:156:22:156:24 | req |
25272540
| src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) | src/advanced-routehandler-registration.js:156:47:156:49 | req |
2541+
| src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:157:28:157:30 | req |
2542+
| src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:157:53:157:55 | req |
25282543
| src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined | src/controllers/handler-in-bulk-require.js:1:45:1:47 | req |
25292544
| src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:20:28:20:30 | req |
25302545
| src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:22:35:22:37 | req |
@@ -2595,10 +2610,15 @@ getRouteHandlerContainerStep
25952610
| src/advanced-routehandler-registration.js:85:15:88:1 | {\\n a: ... (req)\\n} | src/advanced-routehandler-registration.js:87:6:87:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:90:20:90:29 | routes3[p] |
25962611
| src/advanced-routehandler-registration.js:104:15:107:1 | {\\n a: ... (req)\\n} | src/advanced-routehandler-registration.js:105:6:105:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:109:20:109:29 | routes4[p] |
25972612
| src/advanced-routehandler-registration.js:104:15:107:1 | {\\n a: ... (req)\\n} | src/advanced-routehandler-registration.js:106:6:106:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:109:20:109:29 | routes4[p] |
2613+
| src/advanced-routehandler-registration.js:122:17:122:25 | new Map() | src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:125:20:125:20 | v |
25982614
| src/advanced-routehandler-registration.js:122:17:122:25 | new Map() | src/advanced-routehandler-registration.js:123:20:123:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:126:14:126:31 | routesMap.get("a") |
2615+
| src/advanced-routehandler-registration.js:122:17:122:25 | new Map() | src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:125:20:125:20 | v |
25992616
| src/advanced-routehandler-registration.js:122:17:122:25 | new Map() | src/advanced-routehandler-registration.js:124:20:124:49 | (req, r ... og(req) | src/advanced-routehandler-registration.js:127:14:127:31 | routesMap.get("b") |
26002617
| src/advanced-routehandler-registration.js:146:16:146:51 | { handl ... efined} | src/advanced-routehandler-registration.js:146:28:146:50 | (req, r ... defined | src/advanced-routehandler-registration.js:147:9:147:25 | handlers.handlerA |
26012618
| src/advanced-routehandler-registration.js:155:18:155:26 | new Map() | src/advanced-routehandler-registration.js:156:21:156:50 | (req, r ... og(req) | src/advanced-routehandler-registration.js:160:14:160:32 | routesMap2.get("c") |
2619+
| src/advanced-routehandler-registration.js:155:18:155:26 | new Map() | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:160:14:160:32 | routesMap2.get("c") |
2620+
| src/advanced-routehandler-registration.js:155:18:155:26 | new Map() | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:161:14:161:38 | routesM ... nown()) |
2621+
| src/advanced-routehandler-registration.js:155:18:155:26 | new Map() | src/advanced-routehandler-registration.js:157:27:157:56 | (req, r ... og(req) | src/advanced-routehandler-registration.js:163:14:163:32 | routesMap2.get("f") |
26022622
| src/controllers/handler-in-bulk-require.js:1:18:1:68 | { path: ... fined } | src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined | src/advanced-routehandler-registration.js:139:33:139:57 | bulkReq ... handler |
26032623
| src/route-collection.js:1:18:4:1 | {\\n a: ... (req)\\n} | src/route-collection.js:2:6:2:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:116:14:116:30 | importedRoutes[p] |
26042624
| src/route-collection.js:1:18:4:1 | {\\n a: ... (req)\\n} | src/route-collection.js:2:6:2:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:118:14:118:29 | importedRoutes.a |

0 commit comments

Comments
 (0)