diff --git a/javascript/ql/lib/change-notes/2023-04-08-NextJS-router-push.md b/javascript/ql/lib/change-notes/2023-04-08-NextJS-router-push.md new file mode 100644 index 000000000000..83547b0bf5a9 --- /dev/null +++ b/javascript/ql/lib/change-notes/2023-04-08-NextJS-router-push.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* `router.push` and `router.replace` in `Next.js` are now considered as XSS sink. diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll index 98d85d887414..de19e549f9c3 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll @@ -231,6 +231,8 @@ module ClientSideUrlRedirect { NextRoutePushUrlSink() { this = NextJS::nextRouter().getAMemberCall(["push", "replace"]).getArgument(0) } + + override predicate isXssSink() { any() } } private class SinkFromModel extends Sink { diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected index d40a652b962f..383f0a8f5808 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected @@ -560,6 +560,25 @@ nodes | 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 | +| react-use-router.js:4:9:4:28 | router | +| react-use-router.js:4:18:4:28 | useRouter() | +| react-use-router.js:8:21:8:26 | router | +| react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:39 | router.query.foobar | +| react-use-router.js:8:21:8:39 | router.query.foobar | +| react-use-router.js:11:24:11:29 | router | +| react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:42 | router.query.foobar | +| react-use-router.js:11:24:11:42 | router.query.foobar | +| react-use-router.js:22:15:22:24 | router | +| react-use-router.js:22:17:22:22 | router | +| react-use-router.js:23:43:23:48 | router | +| react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:61 | router.query.foobar | +| react-use-router.js:23:43:23:61 | router.query.foobar | | react-use-state.js:4:9:4:49 | state | | react-use-state.js:4:9:4:49 | state | | react-use-state.js:4:10:4:14 | state | @@ -1700,6 +1719,28 @@ edges | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | | react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | | react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | +| react-use-router.js:4:9:4:28 | router | react-use-router.js:8:21:8:26 | router | +| react-use-router.js:4:9:4:28 | router | react-use-router.js:11:24:11:29 | router | +| react-use-router.js:4:18:4:28 | useRouter() | react-use-router.js:4:9:4:28 | router | +| react-use-router.js:8:21:8:26 | router | react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:32 | router.query | 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 | +| react-use-router.js:8:21:8:32 | router.query | 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 | +| react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:4:18:4:28 | useRouter() | +| react-use-router.js:11:24:11:29 | router | react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:35 | router.query | 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 | +| react-use-router.js:11:24:11:35 | router.query | 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 | +| react-use-router.js:22:15:22:24 | router | react-use-router.js:23:43:23:48 | router | +| react-use-router.js:22:17:22:22 | router | react-use-router.js:22:15:22:24 | router | +| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:54 | router.query | 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 | +| react-use-router.js:23:43:23:54 | router.query | 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 | +| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:22:17:22:22 | router | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | @@ -2386,6 +2427,10 @@ edges | 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 | | 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 | | 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 | +| 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 | +| 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 | +| 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 | +| 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 | | 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 | | 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 | | 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 | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected index 5e5010d126e1..5dbee3f5b438 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected @@ -572,6 +572,25 @@ nodes | 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 | +| react-use-router.js:4:9:4:28 | router | +| react-use-router.js:4:18:4:28 | useRouter() | +| react-use-router.js:8:21:8:26 | router | +| react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:39 | router.query.foobar | +| react-use-router.js:8:21:8:39 | router.query.foobar | +| react-use-router.js:11:24:11:29 | router | +| react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:42 | router.query.foobar | +| react-use-router.js:11:24:11:42 | router.query.foobar | +| react-use-router.js:22:15:22:24 | router | +| react-use-router.js:22:17:22:22 | router | +| react-use-router.js:23:43:23:48 | router | +| react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:61 | router.query.foobar | +| react-use-router.js:23:43:23:61 | router.query.foobar | | react-use-state.js:4:9:4:49 | state | | react-use-state.js:4:9:4:49 | state | | react-use-state.js:4:10:4:14 | state | @@ -1762,6 +1781,28 @@ edges | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | | react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | | react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | +| react-use-router.js:4:9:4:28 | router | react-use-router.js:8:21:8:26 | router | +| react-use-router.js:4:9:4:28 | router | react-use-router.js:11:24:11:29 | router | +| react-use-router.js:4:18:4:28 | useRouter() | react-use-router.js:4:9:4:28 | router | +| react-use-router.js:8:21:8:26 | router | react-use-router.js:8:21:8:32 | router.query | +| react-use-router.js:8:21:8:32 | router.query | 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 | +| react-use-router.js:8:21:8:32 | router.query | 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 | +| react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:4:18:4:28 | useRouter() | +| react-use-router.js:11:24:11:29 | router | react-use-router.js:11:24:11:35 | router.query | +| react-use-router.js:11:24:11:35 | router.query | 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 | +| react-use-router.js:11:24:11:35 | router.query | 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 | +| react-use-router.js:22:15:22:24 | router | react-use-router.js:23:43:23:48 | router | +| react-use-router.js:22:17:22:22 | router | react-use-router.js:22:15:22:24 | router | +| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query | +| react-use-router.js:23:43:23:54 | router.query | 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 | +| react-use-router.js:23:43:23:54 | router.query | 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 | +| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:22:17:22:22 | router | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | | react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-router.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-router.js new file mode 100644 index 000000000000..321821cd7291 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-router.js @@ -0,0 +1,25 @@ +import { useRouter } from 'next/router' + +export function nextRouter() { + const router = useRouter(); + return ( +
+ { + router.push(router.query.foobar) // NOT OK + }}>Click to XSS 1 + { + router.replace(router.query.foobar) // NOT OK + }}>Click to XSS 2 + { + router.push('/?foobar=' + router.query.foobar) // OK + }}>Safe Link +
+ ) +} + +import { withRouter } from 'next/router' + +function Page({ router }) { + return router.push(router.query.foobar)}>Click to XSS 3 // NOT OK +} +export const pageWithRouter = withRouter(Page);