1429788 - Web storage file size limitation is unsuitable

Status: RESOLVED WONTFIX

(Core :: Storage: localStorage & sessionStorage, defect, P2)

Description

[rnsztng]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

Firefox has a problem on Web Storage.

For example, a Web site included follow JavaScript.

------------
var storage = sessionStorage;
function set (){
var value = "AAAAA.......A";     // 5MB text data

var i=0;
while(true){
storage.setItem(i,value); 
i++;
}
------------

And access the web site.


Actual results:

FireFox consumes huge memory and then consumes swap.
Then, Firefox crashes.
This problem leads to Dos.


Expected results:

FireFox should have limit for file size of Web Storage.
This page (https://johnresig.com/blog/dom-storage/) says that "5120KB is the default storage size for an entire domain" in FireFox.
But, "5120KB" seems to be the limit of storage for one entry.
You should limit the total size of all entries that Web Storage can use for the entire browser or for each origin.
For example, the total capacity (occupied by all entries) should be limited to 100 MB. At least FireFox will not crash due to memory consumption.

Comment 1

[Daniel Veditz [:dveditz]]

We don't need to hide a DoS.

Jan: is our quota manager not working here?

Reporter: you set this to "54 Branch". What version of Firefox were you testing? Was it really the unsupported 54 version?

Comment 2

[Jan Varga [:janv]]

Session storage is memory only, it's not persisted to disk and it's not managed by quota manager.
There's a 5 MB limit for whole eTLD+1 domain, not just for one entry.

I guess there's an internal problem in implementation that leads to OOM condition.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

rnsztng, is that the exact test code (modulo all the A's) that you're using?  The setItem calls should be throwing, but if the exceptions are caught, a JS/XPConnect GC issue might be possible.

Can you check that the value for your "dom.storage.default_quota" is the default of 5120 by opening the URL "about.config" and typing the pref name in to filter for it?

Also, are you really using Firefox 54 as the bug fields suggest?  If so, can you try on a more recent Firefox build like the stable 57 release?  If I create a string that's 5,000,000 bytes in length and try and setItem it twice, the second try fails for me on current Firefox nightly, but the underlying code hasn't changed for several releases.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Whoops, I mean "about:config", not about.config.  Sorry!

Comment 5

[rnsztng]

Hello. Sorry for delay.

> We don't need to hide a DoS.
> 
> Jan: is our quota manager not working here?
> 
> Reporter: you set this to "54 Branch". What version of Firefox were you
> testing? Was it really the unsupported 54 version?

> Can you check that the value for your "dom.storage.default_quota" is the default of 5120 by opening the URL "about.config" and typing the pref name in to filter for it?
> 
> Also, are you really using Firefox 54 as the bug fields suggest?  If so, can you try on a more recent Firefox build like the stable 57 release?  If I create a string that's 5,000,000 bytes in length and try and setItem it twice, the second try fails for me on current Firefox nightly, but the underlying code hasn't changed for several releases.

dom.storage.default_quota was default. => 5120.
I found this issue on Firefox54, but issue was also on Firefox Nightly 59.0a1 (2018-01-12) (64 bit).

----------------------
> Session storage is memory only, it's not persisted to disk and it's not
> managed by quota manager.
> There's a 5 MB limit for whole eTLD+1 domain, not just for one entry.
> 
> I guess there's an internal problem in implementation that leads to OOM
> condition.

This issue was on SessionStorage and LocalStorage.
For example, I save the 50 MB data in Local storage. Of course, browser fails to save the data, and the console outputs exception named "NS_ERROR_DOM_QUOTA_REACHED"  in developer tools.

For example again, I save the 50,000 MB data in Local storage. browser consumes huge memory before the console outputs exception.

I found testing page for Web Storage. This is better than my code and we can try instantly.
Please access the below URL.

https://demo.agektmr.com/storage/

Please select the [Fill storage] button, and set [Chunk size: 5.0 MB], [Quantity: 10000]. The total is 48.8GB.
Click the [Fill] button,and then we can see that Firefox process consumes memory.

This issue did not reproduce in Firefox on Virtual Machine. At least, Windows 10 on VirtualBox did not reproduce.

I tried the test using Firefox Nightly 59.0a1 on macOS. 
https://imgur.com/a/LAPVC
Sorry for showing non-English pic. The column written in "メモリ" is memory. Clearly, it is abnormal. After that, more usage memory is increasing.

> I guess there's an internal problem in implementation that leads to OOM condition.
I think so, too.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

(In reply to rnsztng from comment #5)
> I found testing page for Web Storage. This is better than my code and we can
> try instantly.
> Please access the below URL.
> 
> https://demo.agektmr.com/storage/

This demo creates a huge transient memory usage spike that causes the OOM.  Reading the logic, using the parameters you suggest it:
- Creates 10,000 5 megabyte Blobs synchronously in a tight loop and stores them in the WebStorage's instance's `queue` array which means they're not eligible for GC.
- Kicks off an async process that uses FileReader instances to read the contents back out of those blobs.  When the FileReader onload fires, it will attempt to perform the setItem() call.  If the call throws because of the quota limit, the `queue` is cleared which will allow GC to happen.  If it doesn't throw, the next FileReader is kicked off.
- Because that FileReader is kicked off after the first blob is created, anything that causes a nested event loop to be spun can allow the FileReader onload to fire and thereby clear the queue array and allow GC to occur.  For example, a modal dialog (possibly including the slow script dialog?), etc.  Any instance where we don't crash is likely to be experiencing such a scenario.

The demo also uses a different key and value for each iteration.  Because different keys are used, the limit is hit.  (If the same key was used but different values, we would expect the resulting DOM events and/or broadcast propagation to potentially cause memory issues.  But since the storage limit is hit, that's not a concern.)

Having double-checked the gecko code and considered the example code, I think in all situations the tests are just creating a normal OOM and the size limits work.  As noted, if the same key was used and different values were used, the OOM could be more interesting because the memory bloat would be occurring outside the page's direct ownership (although strictly as a result of flow control issues; at low mutation rates this doesn't matter), but the root cause is still the same.  I do think we want to do a better job of attempting to terminate script/pages that trigger an OOM rather than letting the OOM kill the browser, but I don't think the storage size limitation is involved here.

I'm not sure how we want to track this in bugzilla.  LocalStorage/SessionStorage as an OOM vector is certainly an interesting edge-case, but again, it's only an issue in that there's no back-pressure to ensure the event pipelines don't back up infinitely.  So maybe we just dupe this to a general OOM bug or something?

Comment 7

[Jens Stutte [:jstutte]]

Recap: As there are many ways to force an DoS through OOM, closing this as WONTFIX.