Address history back button security issue (full page loads)

[nicholaspufal]

Disclaimer: I'm open to ideas on how to address this in a better way here. This isn't the same as the popstate custom handler people have proposed in other issues/PRs.

Summary of the problem

From a security standpoint, there are two situations where Inertia restores props from its cache (i.e. window.history.state) that are concerning at the moment:

1. During client-side navigation (e.g. after clicking on an InertiaLink component) a user goes back in the browser's history

   • This relies on the popstate API to restore the app's state, has been discussed in a few issues in this project, and it's not what this PR is about. Still warrants attention though.

2. After full page loads a user goes back in the browser's history

   • This is what this PR is about

👨‍💻 This is the code affecting behavior related to number 2 from above: https://github.com/inertiajs/inertia/blob/master/packages/core/src/router.ts#L157-L163

• page here represents fresh Inertia props. When a backend request is performed, and Inertia props are provided via the rendered HTML, this is what page here includes
• window.history.state on the other hand represents stale props. This is the state of the Inertia props prior to what page contains.

The issue is that the code is always giving preference to those stale props. Because Inertia props are the bread and butter of Inertia, they often contain sensitive information from the users, information which is cleared when they log out, but because Inertia pulls those from its cache (without any sort of configuration around this behaviour, or without checking if page's props may be fresher) sensitive information may be leaked to third-parties in Inertia apps.

Reproducing this problem

from an Inertia app

1. User A is logged in and at the path /foobar
2. User A signs out and this causes a full /foobar page load
3. User A leaves the shared computer
4. User B goes back in the browser's history
5. User B can see all sensitive information from User A

[zcuric]

@nicholaspufal Hi, does this also fixes the undefined issue that I've address here #1786?

[nicholaspufal]

@zcuric I'm not sure, I have never encountered that scenario tbh. If you have a solid way to reproduce that, could you use my branch and test it out please?

[zcuric]

@nicholaspufal It is fixed, I just tested it in my project!

So to reproduce it, on the master branch:

1. Go to a homepage, for example
2. Go to another page
3. Click back and you should see something like this:

This is caused by the same issue you fixed. My PR #1786 addresses just above mentioned issue. Your PR needs to be merged, especially if it's a security issue. I hope someone will look at this. 🙏🏻

[Hasan-Mir]

Just wanted to mention that this PR fixes the useRemember's bug https://github.com/inertiajs/inertia/issues/1766 as well.

[nicholaspufal]

@jessarcher @reinink given comments above, please let me know if there is anything you need me to do to expedite this — and if you are ok with the approach here.

Thank you!

[alucic]

@jessarcher Please let me know if I can assist somehow to get this PR out, this would help us a lot. Every single back button is throwing an error and spamming our Sentry channel. The error is generic Error so it is hard to archive it or ignore it. Thanks

[zcuric]

Just so you know, we used https://github.com/ds300/patch-package to apply patches to @inertiajs/core to fix these two issues. We applied, this PR and #1786.

[WillRy]

This security flaw is serious, it even prevented me from using inertia js in a company. I was banned from using it due to this security issue.

@jessarcher @reinink

There are other issues that mention the problem, but all the solutions so far are workarounds.

Look this others issues:

• https://github.com/inertiajs/inertia/issues/247
• https://github.com/inertiajs/inertia/issues/102

[stefanocurnis]

Please guys, this is serious. Any chance to bring attention to this via @lepikhinb ?

[reinink]

Hey folks, just want to let you know that this is on our radar and we've been having discussions internally on how to best handle signing out of an Inertia.js app and clearing all the client-side history.

Unfortunately the browser's history APIs don't make this easy — there is no way to clear history state data. From the research I've been doing I think our best bet is to simply track a unique visit ID in the history state and then save the actual page data in localStorage, and then provide some type of mechanism within the Inertia router to clear all this history when logging out of your app. Possibly something like this:

import { router } from `@inertiajs/react`

router.clearHistory()

Then when a user goes back or forward in history Inertia can check if that unique visit ID exists in the history state (in local storage), and if it doesn't it can perform a full inertia visit — and if the user has been logged out they'll then be redirected to the login page.

We might even be able to add some sort of server-side header that does this client-side history clearing automatically doing something like this:

public function logout(Request $request): RedirectResponse
{
    Auth::logout();

    // ...

    Inertia::clearHistory();

    return redirect('/');
}

That's my working idea right now at least. Need to actually prototype this and see how it works. It's a little scary switching from history state to local storage for page data storage, but maybe it will just work 🤞

As for this particular PR, I'm not 100% sure it's the right change. Yes, it is a partial fix to the security concerns being addressed in #102 and #247, but it's not the right solution for non-auth related situations where you do want to use the data in the history state when going back or forward in your history. Just because it's a "full page" history visit doesn't necessarily mean it should read fresh data from the server. Anyway, going to leave this PR open either way as I may change my mind on this while working on these changes.

[WillRy]

Hi everyone, I think Jonathan Reinink's solution is a good way to go.

Here's a suggestion: inertia could do the "clearHistory" automatically when detecting a 401, which indicates an expired session.

[nicholaspufal]

@reinink I love the idea of setting a standard for the backend to indicate to the frontend that its caching should be purged, and the switch to local storage makes total sense to me.

Personally I think it's important to have a sane default for this behavior (like @WillRy's suggestion above, 401s may be a good start). Having the 401 default be overridden by some custom configuration would be the cherry on top, for teams that prefer to leverage a special header or some other key like you suggested.

[svengt]

@reinink Sounds like a great solution! sessionStorage might also be a great fit for this implementation as it automatically clears after closing a tab?

[deleteme]

Would it be beneficial to encrypt page props with a key associated with the session? Upon log out, without the key, stale page props would be inaccessible.

Having a method to clear history state would still be valuable, to prevent the accumulation of useless state and avoid a QUOTA_EXCEEDED_ERR exception.