Fossil Forum

Serious /wikiedit and /fileedit cross-repo (same site) data leak flaw in 2.12
Login

Serious /wikiedit and /fileedit cross-repo (same site) data leak flaw in 2.12

Serious /wikiedit and /fileedit cross-repo (same site, same browser) data leak flaw in 2.12

(1.2) By Stephan Beal (stephan) on 2020-08-18 20:02:37 edited from 1.1 [source]

Data Leak Vulnerability in /wikieedit and /fileedit in 2.12

Edit: updated to tone down the severity, as the problem cannot leak data between multiple users, just edits made across multiple repos by the same user in the same browser profile instance.

In the course of responding to /forumpost/0f56c9edd9 it was, much to my dismay, discovered that we have a serious flaw in the /wikiedit and /fileedit pages in 2.12 which can cause a repo to "leak" edits to other repos being hosted on the "origin" (same domain and port number). The leak only applies to a single browser profile installation, i.e. the same user. Edits made by two users, or across two browsers, will not be visible to each other.

The fix is trivial:

https://fossil-scm.org/fossil/info/5b9a4c90594d8ea6

and simply disables the internal selection of localStorage as a storage option.

If you work with multiple repos on the same origin, please update your 2.12 releases to this version. In parallel, i will work with Richard to get a 2.12.1 out the door ASAP. :( The checkin for that release will be made as soon as this post is submitted (i want to link to this in the commit message).

Note that upgrading to this change will lose any edits currently stashed in localStorage, so if you have any to apply, do so before upgrading.

ChiselApp is not affected - i have it on good authority that they will be upgrading in the coming weeks, but don't yet have 2.12.

Likewise, versions earlier than 2.12 are not affected, as they apply only to features added in 2.12.

Details and Symptoms of the Problem

When /wikiedit and /fileedit believe you have made changes to the page/file you are working on in your browser, they stuff that into the fossil.storage JavaScript API so that you don't lose the edits if you reload the page, your browser crashes, or whatever. fossil.storage, in 2.12, checks for the longest-living storage option of:

  1. window.localStorage
  2. window.sessionStorage
  3. An API-compatible transient in-memory object which is lost when reloading the page.

The fix linked to above simply skips step (1), choosing sessionStorage, if available, which is linked to the tab in question and survives reloads and navigation away from the page, so long as that tab remains open. Thus edits are still persistent, to some degree, but (A) not as persistent as before and (B) they cannot leak outside of that specific tab.

However: sessionData is also affected by this, but less dramatically: only when the user switches to a different repository from the same origin within the same "session" (which is limited to a single browser tab). Two tabs opened at a single origin at the same time, whether it's the same repo or not, do not share this state, so cannot see each other's edits. Closing a tab immediately eliminates the sessionStorage-stored state.

Problem Origin, Facepalm, and Apologies

You have my sincerest apologies for this - it's 100% my fault.

Seriously, this guts me. (Edit: it's not nearly as bad as i initially thought, so...) This is seriously embarrassing. The underlying cause for it is that my understanding of "origin," in the context of localStorage scoping, was fundamentally flawed. The secondary cause was that all test edits on my public test/demo repo were performed on a single repo, so there was no opportunity to witness the cross-repo leaking of the edits in other repos from the same "origin." The line of discussion in the above-mentioned thread brought that into question and, sure enough, a quick multi-repo test on the same server easily reveals that edits do cross repo boundaries via localStorage.

Feature or bug? Yes, but most definitely potentially more of a bug than a feature. (There are contexts where that would indeed be extremely useful, but it was never the intent to share this state across different repo instances.)

Please accept my apologies for this - it was an inexcusable breach.

(2) By Chris (crustyoz) on 2020-08-18 18:54:37 in reply to 1.0 [link] [source]

Stephen said:

Please accept my apologies for this - it was an inexcusable breach.

Apologies not necessary. And definitely not "an inexcusable breach". Are you into self-flagellation as well? :-)

As the original error reporter, I must say that it is truly a long-tail event that might not have been detected for years. Now I need to review my own purchasing application code for the same issue :-(

Chris

(3) By Stephan Beal (stephan) on 2020-08-18 19:02:25 in reply to 2 [link] [source]

Apologies not necessary.

i appreciate that, but i'm definitely not going to sleep tonight :/.

Now I need to review my own purchasing application code for the same issue :-(

Okay, now i don't feel so bad about it ;). Even so... not gonna sleep.

(4) By Stephan Beal (stephan) on 2020-08-18 19:30:20 in reply to 1.1 [link] [source]

"Great news, everyone!"

This leak is not nearly as bad as my imagination made it out to be. The leak happens, but only across a single browser installation and browser-level user profile. Multiple independent repos being used by different browser instances (i.e. different users) will not share data. This affects, in principle, only a single user in their browser of choice, but across any number of repos served by the same origin.

It's still a leak - it makes data from one repository visible in another - but it does not cross user boundaries. In my panic, i was thinking that it could cross user boundaries, but that is definitely not the case.

@Chris: for your purchasing app that might actually qualify as a feature, not a bug. It's still worth checking out, though. Okay, i may be able to sleep tonight, after all.

(7) By Offray (offray) on 2020-08-21 17:53:24 in reply to 4 [link] [source]

It's still a leak - it makes data from one repository visible in another - but it does not cross user boundaries. In my panic, i was thinking that it could cross user boundaries, but that is definitely not the case.

[...] Okay, i may be able to sleep tonight, after all.

Good to know both things, that Fossil is nos severely affected and, more importantly, that you are sleeping well.

These are times to care about healthy communities in the broader sense of the word.

Keep the good work and sleep well.

Offray

(5) By Stephan Beal (stephan) on 2020-08-18 21:08:20 in reply to 1.2 [link] [source]

Update about localStorage and sessionStorage support...

As is reflected in my subsequent edits of the top post:

  1. sessionStorage has the same "leak" as localStorage, but in a far more limited form (see that post for details).
  2. The "leak" is limited to a single combination of origin and browser profile, meaning that no two users will ever see each other's edits, but it was possible for a single user in a single browser to see edits in unrelated repositories living under the same origin.

My initial over-reaction was to deactivate localStorage, but that only shrunk the scope of the issue because of point (1).

The latest trunk and the pending 2.12.1 release now reinstate localStorage (yeah!) but take steps to sandbox access to it on a per-repo basis, so no two repos, even on the same origin/browser profile combination, will use each other's state. To be clear, the storage is still shared, because that's how both of those storage options work, and that is, for the most part, a feature. A client may look in the "storage" tab of the browser's dev tools to see the storage used by all repos from the same origin. What this change does, however, is modify the higher-level storage abstraction API so that any given repository will only ever read, write, or remove storage keys which have its own repo-specific prefix.

Whew. What a night.

(6) By Richard Hipp (drh) on 2020-08-19 00:28:09 in reply to 5 [link] [source]

Technical change: I renamed the branch to "branch-2.12". The name "version-2.12.1" won't work for a branch, because that is the tag we are going to stick on the specific check-in for the 2.12.1 release, when it occurs.