Skip to content

Commit 8cb54b7

Browse files
erik-krogherik-krogh
authored
Merge pull request #12787 from tyage/add-router-sink
JS: Add New XSS sink - Next.js router.push/replace
2 parents b5b0d60 + 40d4758 commit 8cb54b7

File tree

5 files changed

+117
-0
lines changed

5 files changed

+117
-0
lines changed

‎javascript/ql/lib/change-notes/2023-04-08-NextJS-router-push.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* `router.push` and `router.replace` in `Next.js` are now considered as XSS sink.

‎javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@ module ClientSideUrlRedirect {
231231
NextRoutePushUrlSink() {
232232
this = NextJS::nextRouter().getAMemberCall(["push", "replace"]).getArgument(0)
233233
}
234+
235+
override predicate isXssSink() { any() }
234236
}
235237

236238
private class SinkFromModel extends Sink {

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,25 @@ nodes
560560
| react-use-context.js:16:26:16:36 | window.name |
561561
| react-use-context.js:16:26:16:36 | window.name |
562562
| react-use-context.js:16:26:16:36 | window.name |
563+
| react-use-router.js:4:9:4:28 | router |
564+
| react-use-router.js:4:18:4:28 | useRouter() |
565+
| react-use-router.js:8:21:8:26 | router |
566+
| react-use-router.js:8:21:8:32 | router.query |
567+
| react-use-router.js:8:21:8:32 | router.query |
568+
| react-use-router.js:8:21:8:39 | router.query.foobar |
569+
| react-use-router.js:8:21:8:39 | router.query.foobar |
570+
| react-use-router.js:11:24:11:29 | router |
571+
| react-use-router.js:11:24:11:35 | router.query |
572+
| react-use-router.js:11:24:11:35 | router.query |
573+
| react-use-router.js:11:24:11:42 | router.query.foobar |
574+
| react-use-router.js:11:24:11:42 | router.query.foobar |
575+
| react-use-router.js:22:15:22:24 | router |
576+
| react-use-router.js:22:17:22:22 | router |
577+
| react-use-router.js:23:43:23:48 | router |
578+
| react-use-router.js:23:43:23:54 | router.query |
579+
| react-use-router.js:23:43:23:54 | router.query |
580+
| react-use-router.js:23:43:23:61 | router.query.foobar |
581+
| react-use-router.js:23:43:23:61 | router.query.foobar |
563582
| react-use-state.js:4:9:4:49 | state |
564583
| react-use-state.js:4:9:4:49 | state |
565584
| react-use-state.js:4:10:4:14 | state |
@@ -1700,6 +1719,28 @@ edges
17001719
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
17011720
| react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name |
17021721
| react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name |
1722+
| react-use-router.js:4:9:4:28 | router | react-use-router.js:8:21:8:26 | router |
1723+
| react-use-router.js:4:9:4:28 | router | react-use-router.js:11:24:11:29 | router |
1724+
| react-use-router.js:4:18:4:28 | useRouter() | react-use-router.js:4:9:4:28 | router |
1725+
| react-use-router.js:8:21:8:26 | router | react-use-router.js:8:21:8:32 | router.query |
1726+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1727+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1728+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1729+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1730+
| react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:4:18:4:28 | useRouter() |
1731+
| react-use-router.js:11:24:11:29 | router | react-use-router.js:11:24:11:35 | router.query |
1732+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1733+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1734+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1735+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1736+
| react-use-router.js:22:15:22:24 | router | react-use-router.js:23:43:23:48 | router |
1737+
| react-use-router.js:22:17:22:22 | router | react-use-router.js:22:15:22:24 | router |
1738+
| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query |
1739+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1740+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1741+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1742+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1743+
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:22:17:22:22 | router |
17031744
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |
17041745
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |
17051746
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |
@@ -2386,6 +2427,10 @@ edges
23862427
| react-native.js:9:27:9:33 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:9:27:9:33 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
23872428
| react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | Cross-site scripting vulnerability due to $@. | react-use-context.js:10:22:10:32 | window.name | user-provided value |
23882429
| react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | Cross-site scripting vulnerability due to $@. | react-use-context.js:16:26:16:36 | window.name | user-provided value |
2430+
| react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:8:21:8:32 | router.query | user-provided value |
2431+
| react-use-router.js:11:24:11:42 | router.query.foobar | react-use-router.js:8:21:8:32 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:8:21:8:32 | router.query | user-provided value |
2432+
| react-use-router.js:11:24:11:42 | router.query.foobar | react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:11:24:11:35 | router.query | user-provided value |
2433+
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:23:43:23:54 | router.query | user-provided value |
23892434
| react-use-state.js:5:51:5:55 | state | react-use-state.js:4:38:4:48 | window.name | react-use-state.js:5:51:5:55 | state | Cross-site scripting vulnerability due to $@. | react-use-state.js:4:38:4:48 | window.name | user-provided value |
23902435
| react-use-state.js:11:51:11:55 | state | react-use-state.js:10:14:10:24 | window.name | react-use-state.js:11:51:11:55 | state | Cross-site scripting vulnerability due to $@. | react-use-state.js:10:14:10:24 | window.name | user-provided value |
23912436
| react-use-state.js:17:51:17:55 | state | react-use-state.js:16:20:16:30 | window.name | react-use-state.js:17:51:17:55 | state | Cross-site scripting vulnerability due to $@. | react-use-state.js:16:20:16:30 | window.name | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,25 @@ nodes
572572
| react-use-context.js:16:26:16:36 | window.name |
573573
| react-use-context.js:16:26:16:36 | window.name |
574574
| react-use-context.js:16:26:16:36 | window.name |
575+
| react-use-router.js:4:9:4:28 | router |
576+
| react-use-router.js:4:18:4:28 | useRouter() |
577+
| react-use-router.js:8:21:8:26 | router |
578+
| react-use-router.js:8:21:8:32 | router.query |
579+
| react-use-router.js:8:21:8:32 | router.query |
580+
| react-use-router.js:8:21:8:39 | router.query.foobar |
581+
| react-use-router.js:8:21:8:39 | router.query.foobar |
582+
| react-use-router.js:11:24:11:29 | router |
583+
| react-use-router.js:11:24:11:35 | router.query |
584+
| react-use-router.js:11:24:11:35 | router.query |
585+
| react-use-router.js:11:24:11:42 | router.query.foobar |
586+
| react-use-router.js:11:24:11:42 | router.query.foobar |
587+
| react-use-router.js:22:15:22:24 | router |
588+
| react-use-router.js:22:17:22:22 | router |
589+
| react-use-router.js:23:43:23:48 | router |
590+
| react-use-router.js:23:43:23:54 | router.query |
591+
| react-use-router.js:23:43:23:54 | router.query |
592+
| react-use-router.js:23:43:23:61 | router.query.foobar |
593+
| react-use-router.js:23:43:23:61 | router.query.foobar |
575594
| react-use-state.js:4:9:4:49 | state |
576595
| react-use-state.js:4:9:4:49 | state |
577596
| react-use-state.js:4:10:4:14 | state |
@@ -1762,6 +1781,28 @@ edges
17621781
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
17631782
| react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name |
17641783
| react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name |
1784+
| react-use-router.js:4:9:4:28 | router | react-use-router.js:8:21:8:26 | router |
1785+
| react-use-router.js:4:9:4:28 | router | react-use-router.js:11:24:11:29 | router |
1786+
| react-use-router.js:4:18:4:28 | useRouter() | react-use-router.js:4:9:4:28 | router |
1787+
| react-use-router.js:8:21:8:26 | router | react-use-router.js:8:21:8:32 | router.query |
1788+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1789+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1790+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1791+
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar |
1792+
| react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:4:18:4:28 | useRouter() |
1793+
| react-use-router.js:11:24:11:29 | router | react-use-router.js:11:24:11:35 | router.query |
1794+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1795+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1796+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1797+
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar |
1798+
| react-use-router.js:22:15:22:24 | router | react-use-router.js:23:43:23:48 | router |
1799+
| react-use-router.js:22:17:22:22 | router | react-use-router.js:22:15:22:24 | router |
1800+
| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query |
1801+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1802+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1803+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1804+
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar |
1805+
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:22:17:22:22 | router |
17651806
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |
17661807
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |
17671808
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-router.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { useRouter } from 'next/router'
2+
3+
export function nextRouter() {
4+
const router = useRouter();
5+
return (
6+
<div>
7+
<span onClick={() => {
8+
router.push(router.query.foobar) // NOT OK
9+
}}>Click to XSS 1</span>
10+
<span onClick={() => {
11+
router.replace(router.query.foobar) // NOT OK
12+
}}>Click to XSS 2</span>
13+
<span onClick={() => {
14+
router.push('/?foobar=' + router.query.foobar) // OK
15+
}}>Safe Link</span>
16+
</div>
17+
)
18+
}
19+
20+
import { withRouter } from 'next/router'
21+
22+
function Page({ router }) {
23+
return <span onClick={() => router.push(router.query.foobar)}>Click to XSS 3</span> // NOT OK
24+
}
25+
export const pageWithRouter = withRouter(Page);

0 commit comments

Comments
 (0)