hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-05 13:45:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor chrome-remote-interface to use type-tracking promise steps

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-03-10 12:27:21 +01:00
parent 3ddfd7ba73
commit 6110f85748
2 changed files with 16 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll
View File

@@ -566,35 +566,24 @@ module ClientRequest {
* The `isPromise` parameter reflects whether the reference is a promise containing
* an instance of `chrome-remote-interface`, or an instance of `chrome-remote-interface`.
*/
private DataFlow::SourceNode chromeRemoteInterface(DataFlow::TypeTracker t, boolean isPromise) {
t.start() and
private DataFlow::SourceNode chromeRemoteInterface(DataFlow::TypeTracker t) {
exists(DataFlow::CallNode call |
call = DataFlow::moduleImport("chrome-remote-interface").getAnInvocation()
|
result = call and isPromise = true
// the client is inside in a promise.
t = PromiseTypeTracking::valueInPromiseTracker() and result = call
or
result = call.getCallback([0 .. 1]).getParameter(0) and isPromise = false
// the client is accessed directly using a callback.
t.start() and result = call.getCallback([0 .. 1]).getParameter(0)
)
or
exists(DataFlow::TypeTracker t2 | result = chromeRemoteInterface(t2, isPromise).track(t2, t))
// standard type-tracking steps
exists(DataFlow::TypeTracker t2 | result = chromeRemoteInterface(t2).track(t2, t))
or
// Simple promise tracking.
exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred |
pred = chromeRemoteInterface(t2, true) and
isPromise = false and
(
t2 = t and
exists(AwaitExpr await | DataFlow::valueNode(await.getOperand()).getALocalSource() = pred |
result.getEnclosingExpr() = await
)
or
t2 = t and
exists(DataFlow::MethodCallNode thenCall |
thenCall.getMethodName() = "then" and pred = thenCall.getReceiver().getALocalSource()
|
result = thenCall.getCallback(0).getParameter(0)
)
)
exists(DataFlow::TypeTracker t2, DataFlow::StepSummary summary |
result = PromiseTypeTracking::promiseStep(chromeRemoteInterface(t2), summary) and
t = t2.append(summary)
)
}
@@ -606,7 +595,7 @@ module ClientRequest {
ChromeRemoteInterfaceRequest() {
exists(DataFlow::SourceNode instance |
instance = chromeRemoteInterface(DataFlow::TypeTracker::end(), false)
instance = chromeRemoteInterface(DataFlow::TypeTracker::end())
|
optionsArg = 0 and
this = instance.getAPropertyRead("Page").getAMemberCall("navigate")

5
javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected
View File

@@ -47,6 +47,8 @@ nodes
| tst.js:61:29:61:35 | tainted |
| tst.js:64:30:64:36 | tainted |
| tst.js:64:30:64:36 | tainted |
| tst.js:68:30:68:36 | tainted |
| tst.js:68:30:68:36 | tainted |
edges
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -89,6 +91,8 @@ edges
| tst.js:58:9:58:52 | tainted | tst.js:61:29:61:35 | tainted |
| tst.js:58:9:58:52 | tainted | tst.js:64:30:64:36 | tainted |
| tst.js:58:9:58:52 | tainted | tst.js:64:30:64:36 | tainted |
| tst.js:58:9:58:52 | tainted | tst.js:68:30:68:36 | tainted |
| tst.js:58:9:58:52 | tainted | tst.js:68:30:68:36 | tainted |
| tst.js:58:19:58:42 | url.par ... , true) | tst.js:58:19:58:48 | url.par ... ).query |
| tst.js:58:19:58:48 | url.par ... ).query | tst.js:58:19:58:52 | url.par ... ery.url |
| tst.js:58:19:58:52 | url.par ... ery.url | tst.js:58:9:58:52 | tainted |
@@ -109,3 +113,4 @@ edges
| tst.js:45:5:45:57 | request ... ainted) | tst.js:14:29:14:35 | req.url | tst.js:45:13:45:56 | 'http:/ ... tainted | The $@ of this request depends on $@. | tst.js:45:13:45:56 | 'http:/ ... tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
| tst.js:61:2:61:37 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:61:29:61:35 | tainted | The $@ of this request depends on $@. | tst.js:61:29:61:35 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
| tst.js:64:3:64:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:64:30:64:36 | tainted | The $@ of this request depends on $@. | tst.js:64:30:64:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
| tst.js:68:3:68:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:68:30:68:36 | tainted | The $@ of this request depends on $@. | tst.js:68:30:68:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |