Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix history state navigation security issue #1996

Open
wants to merge 6 commits into
base: v1
Choose a base branch
from

Conversation

gitbugr
Copy link

@gitbugr gitbugr commented Oct 5, 2024

Implements changes proposed in the following comment #1784 (comment) back in May, without the server-side header idea (not against it, though I thought it was out of scope for this, and I didn't want to touch more than I needed to.), and using window.sessionStorage instead of window.localStorage.

To summarise the comment and what this PR does

  • Moves page cached history from being stored in window.history.state (which cannot be cleared, presenting a potential security issue if private information is stored in page props.), to window.sessionStorage, which lives for the lifetime of the tab (though most browsers ignore security concerns by allowing users to restore sessions..) and can be cleared in JS.
  • Added a public method to the router, router.clearHistory(), for clearing history state.

Related tickets/pull-requests:

#1784
#247
#102

@gitbugr gitbugr force-pushed the GH-247/fix_history_state_navigation_security_issue branch from ff3abc4 to 0d094ad Compare October 5, 2024 14:48
@gitbugr gitbugr force-pushed the GH-247/fix_history_state_navigation_security_issue branch from 513acad to 119db3d Compare October 5, 2024 15:23
@gitbugr
Copy link
Author

gitbugr commented Oct 5, 2024

Not sure what's up with the tests, here, the error modal test fails, but when running locally with the gui they all pass?

I've got it. Was using crypto.randomUUID() which isn't compatable with the current cypress electron version.

@gitbugr gitbugr force-pushed the GH-247/fix_history_state_navigation_security_issue branch from 4d7d6f9 to 97889b1 Compare October 6, 2024 12:07
@gitbugr
Copy link
Author

gitbugr commented Oct 6, 2024

I've added somewhat of a guard against using more than ~1MB (+/- the size of one Page - somewhat arbitrarily chosen. Max is 5MB, but inertia should never fill that and prevent devs from being able to use it themselves.) of session storage for history. Upon reaching > 1MB, will begin to shift items out of session storage to prevent unlikely but possible QUOTA_EXCEEDED_ERR mentioned in #1784 (comment).

@reinink reinink changed the base branch from master to v1 October 9, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant