Skip to content

Commit 5c388c5

Browse files
erik-kroghatorralba
authored andcommitted
fix that the TypeTracker was unrestricted for the base-case of nonFirstLocationType
1 parent e02b67a commit 5c388c5

File tree

3 files changed

+16
-2
lines changed

3 files changed

+16
-2
lines changed

javascript/ql/lib/semmle/javascript/DOM.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,11 @@ module DOM {
465465
*/
466466
private DataFlow::SourceNode nonFirstLocationType(DataFlow::TypeTracker t) {
467467
// One step inlined in the beginning.
468-
result = any(DataFlow::Node n | n.hasUnderlyingType("Location")).getALocalSource().track(_, t)
468+
exists(DataFlow::TypeTracker t2 |
469+
result =
470+
any(DataFlow::Node n | n.hasUnderlyingType("Location")).getALocalSource().track(t2, t) and
471+
t2.start()
472+
)
469473
or
470474
exists(DataFlow::TypeTracker t2 | result = nonFirstLocationType(t2).track(t2, t))
471475
}

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,14 @@ nodes
220220
| typed.ts:29:33:29:43 | redirectUri |
221221
| typed.ts:47:25:47:34 | loc.search |
222222
| typed.ts:47:25:47:34 | loc.search |
223+
| typed.ts:48:26:48:36 | loc2.search |
224+
| typed.ts:48:26:48:36 | loc2.search |
223225
| typed.ts:51:24:51:34 | redirectUri |
224226
| typed.ts:52:33:52:43 | redirectUri |
225227
| typed.ts:52:33:52:43 | redirectUri |
228+
| typed.ts:55:25:55:35 | redirectUri |
229+
| typed.ts:56:33:56:43 | redirectUri |
230+
| typed.ts:56:33:56:43 | redirectUri |
226231
edges
227232
| electron.js:4:12:4:22 | window.name | electron.js:7:20:7:29 | getTaint() |
228233
| electron.js:4:12:4:22 | window.name | electron.js:7:20:7:29 | getTaint() |
@@ -419,8 +424,12 @@ edges
419424
| typed.ts:28:24:28:34 | redirectUri | typed.ts:29:33:29:43 | redirectUri |
420425
| typed.ts:47:25:47:34 | loc.search | typed.ts:51:24:51:34 | redirectUri |
421426
| typed.ts:47:25:47:34 | loc.search | typed.ts:51:24:51:34 | redirectUri |
427+
| typed.ts:48:26:48:36 | loc2.search | typed.ts:55:25:55:35 | redirectUri |
428+
| typed.ts:48:26:48:36 | loc2.search | typed.ts:55:25:55:35 | redirectUri |
422429
| typed.ts:51:24:51:34 | redirectUri | typed.ts:52:33:52:43 | redirectUri |
423430
| typed.ts:51:24:51:34 | redirectUri | typed.ts:52:33:52:43 | redirectUri |
431+
| typed.ts:55:25:55:35 | redirectUri | typed.ts:56:33:56:43 | redirectUri |
432+
| typed.ts:55:25:55:35 | redirectUri | typed.ts:56:33:56:43 | redirectUri |
424433
#select
425434
| electron.js:7:20:7:29 | getTaint() | electron.js:4:12:4:22 | window.name | electron.js:7:20:7:29 | getTaint() | Untrusted URL redirection depends on a $@. | electron.js:4:12:4:22 | window.name | user-provided value |
426435
| react.js:10:60:10:81 | documen ... on.hash | react.js:10:60:10:81 | documen ... on.hash | react.js:10:60:10:81 | documen ... on.hash | Untrusted URL redirection depends on a $@. | react.js:10:60:10:81 | documen ... on.hash | user-provided value |
@@ -485,3 +494,4 @@ edges
485494
| typed.ts:8:33:8:43 | redirectUri | typed.ts:4:22:4:36 | location.search | typed.ts:8:33:8:43 | redirectUri | Untrusted URL redirection depends on a $@. | typed.ts:4:22:4:36 | location.search | user-provided value |
486495
| typed.ts:29:33:29:43 | redirectUri | typed.ts:25:25:25:34 | loc.search | typed.ts:29:33:29:43 | redirectUri | Untrusted URL redirection depends on a $@. | typed.ts:25:25:25:34 | loc.search | user-provided value |
487496
| typed.ts:52:33:52:43 | redirectUri | typed.ts:47:25:47:34 | loc.search | typed.ts:52:33:52:43 | redirectUri | Untrusted URL redirection depends on a $@. | typed.ts:47:25:47:34 | loc.search | user-provided value |
497+
| typed.ts:56:33:56:43 | redirectUri | typed.ts:48:26:48:36 | loc2.search | typed.ts:56:33:56:43 | redirectUri | Untrusted URL redirection depends on a $@. | typed.ts:48:26:48:36 | loc2.search | user-provided value |

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/typed.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@ export class WeirdTracking {
5353
}
5454

5555
private doRedirect2(redirectUri: string) {
56-
window.location.replace(redirectUri); // NOT OK - but not flagged [INCONSISTENCY]
56+
window.location.replace(redirectUri); // NOT OK - and correctly flagged
5757
}
5858
}

0 commit comments

Comments
 (0)