search

remix-run / history

Manage session history with JavaScript
MIT License
8.29k stars 960 forks source link

Fortify Security Scan - Critical Issue: Cross-Site Scripting #883

Closed SainadhSai closed 3 years ago

SainadhSai commented 3 years ago

Critical Issue identified by fortify scan with react-router-dom.js(history.js) file, below is the detailed summary..,

The method handleHashChange() method sends unvalidated data to a web browser, which can result in the browser executing malicious code. Sending unvalidated data to a web browser can result in the browser executing malicious code.

image

StringEpsilon commented 3 years ago

I'm not sure this is correct.

handleHashChange is only used as an event listener for HashChangeEvent (see v4 branch, createBrowserHistory.js, line 273 / 278) - after the URL already changed. History would not provide any new malicious payload in that case.

getHistoryState() only ever reads from the browser. getDOMLocation() also only reads. handlePop() has a path that sends data to the browser via revertPop() (line 122), but it only does so via globalHistory.go(-1);

Does that fortify scan have any more info on where exactly the bad assignment / API call supposedly happens?

Also, there was a false positive from fortify here before: https://github.com/ReactTraining/history/issues/881

mjackson commented 3 years ago

It's unclear which piece of this code is "sending data to a web browser". This method is reading history.state and window.location, not setting them. Seems like a false positive to me. Closing.