JS: Add New XSS sink - Next.js router.push/replace

[tyage]

Made Next.js router's push/replace methods as XSS sink.
They should be XSS sink because XSS occurs when their argument is javascript:alert(1).

https://nextjs.org/docs/api-reference/next/router#routerpush
https://nextjs.org/docs/api-reference/next/router#routerreplace

BTW, those methods are considered as XSS sink in the test code of ClientSideUrlRedirect but they are not actually treated as XSS sink.

codeql/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/react.js

Lines 26 to 35 in 7f9b855

	export function nextRouter() {
	const router = useRouter();
	return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 1</span>
	}
	import { withRouter } from 'next/router'
	function Page({ router }) {
	return <span onClick={() => router.push(document.location.hash.substr(1))}>Click to XSS 2</span>
	}

[erik-krogh]

Looks good, thanks 👍

I noticed that HistoryWriteUrlSink could use the same treatment, I'll do that myself in a followup PR.

[tyage]

@erik-krogh
Thanks for merging!

I didn't aware that @npm/history can be a sink because I tested with history v4.
It seems history v4 is safe but history v5 is not safe 😓

history v4
https://stackblitz.com/edit/js-xervrs

history v5
https://stackblitz.com/edit/js-q6v4fa

remix-run/history#811

[erik-krogh]

I didn't aware that @npm/history can be a sink because I tested with history v4. It seems history v4 is safe but history v5 is not safe 😓

Interesting....
I just tested with the latest version to confirm that it was a thing.

I don't think I'll add a version check to the @npm/history sink, as upgrading to the latest version can make you vulnerable.