From b21dbdb20d011564dce1a31d505fe4b8fbd96684 Mon Sep 17 00:00:00 2001 From: Jacob Date: Mon, 12 Nov 2018 11:19:01 -0600 Subject: [PATCH] fix: bypass DOM storage quota (#15596) --- patches/common/chromium/.patches | 2 +- .../common/chromium/dom_storage_limits.patch | 98 +++++++++++++++++++ patches/common/chromium/dom_storage_map.patch | 23 ----- spec/chromium-spec.js | 14 +++ 4 files changed, 113 insertions(+), 24 deletions(-) create mode 100644 patches/common/chromium/dom_storage_limits.patch delete mode 100644 patches/common/chromium/dom_storage_map.patch diff --git a/patches/common/chromium/.patches b/patches/common/chromium/.patches index 8843abea59703..cef44f416af64 100644 --- a/patches/common/chromium/.patches +++ b/patches/common/chromium/.patches @@ -11,7 +11,7 @@ browser_plugin_wheel.patch can_create_window.patch compositor_delegate.patch disable_hidden.patch -dom_storage_map.patch +dom_storage_limits.patch frame_host_manager.patch net_url_request_job.patch no_stack_dumping.patch diff --git a/patches/common/chromium/dom_storage_limits.patch b/patches/common/chromium/dom_storage_limits.patch new file mode 100644 index 0000000000000..7cd7ec2d334c8 --- /dev/null +++ b/patches/common/chromium/dom_storage_limits.patch @@ -0,0 +1,98 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jacob Quant +Date: Tue, 6 Nov 2018 15:26:00 -0600 +Subject: dom_storage_limits.patch + +This patch circumvents the restriction on DOM storage objects, +namely `localStorage` and `sessionStorage`, which chromium otherwise +limits to approximately 10MiB. + +That restriction originates from a recommendation +[in the Web Storage API specification](https://html.spec.whatwg.org/multipage/webstorage.html#disk-space-2) +that is motivated by the concern that hostile code could abuse this +feature to exhaust available storage capacity. +However, in the case of Electron, where the application developers +have control over all of the code being executed, +this safety precaution becomes a hindrance that does not add much value. +For example, if a malicious developer wanted to consume disk space +on a victim's machine they could do so via Node's native file system API. + +By disabling this restriction or increasing the quota, +electron application developers can use `localStorage` +as their application's "back end", without being having +to limit the amount of data stored to 10MiB. + +There may still be some benefit to keeping this restriction for applications that load remote content. +Although all remote data should be from a trusted source and transferred using +a secure channel, it is nevertheless advisable to include additional layers of protection +to mitigate risks associated with potential compromise of those other technologies. +With that in mind, an acceptable alternative to disabling the limit at compile-time +(as this patch currently does) would be to instead allow it to be disabled at run-time +for a given `BrowserWindow` via a `webPreferences` option, +similar to [`nodeIntegration`](https://electronjs.org/docs/tutorial/security#2-disable-nodejs-integration-for-remote-content). + +diff --git a/content/common/dom_storage/dom_storage_map.cc b/content/common/dom_storage/dom_storage_map.cc +index fd088fb170be..b90b6cf9132d 100644 +--- a/content/common/dom_storage/dom_storage_map.cc ++++ b/content/common/dom_storage/dom_storage_map.cc +@@ -185,10 +185,12 @@ bool DOMStorageMap::SetItemInternal(MapType* map_type, + size_t new_item_size = size_in_storage(key, value); + size_t new_storage_used = storage_used_ - old_item_size + new_item_size; + ++#if 0 + // Only check quota if the size is increasing, this allows + // shrinking changes to pre-existing files that are over budget. + if (new_item_size > old_item_size && new_storage_used > quota_) + return false; ++#endif + + (*map_type)[key] = value; + ResetKeyIterator(); +diff --git a/content/common/dom_storage/dom_storage_types.h b/content/common/dom_storage/dom_storage_types.h +index e87afe5b8ee0..61c9a0dfff60 100644 +--- a/content/common/dom_storage/dom_storage_types.h ++++ b/content/common/dom_storage/dom_storage_types.h +@@ -21,6 +21,7 @@ typedef std::map DOMStorageValuesMap; + + // The quota for each storage area. + // This value is enforced in renderer processes and the browser process. ++// However, Electron's dom_storage_limits.patch removes the code that checks this limit. + const size_t kPerStorageAreaQuota = 10 * 1024 * 1024; + + // In the browser process we allow some overage to +diff --git a/content/renderer/dom_storage/dom_storage_cached_area.cc b/content/renderer/dom_storage/dom_storage_cached_area.cc +index 402c27727ff1..f5908a1c55f9 100644 +--- a/content/renderer/dom_storage/dom_storage_cached_area.cc ++++ b/content/renderer/dom_storage/dom_storage_cached_area.cc +@@ -53,11 +53,13 @@ bool DOMStorageCachedArea::SetItem(int connection_id, + const base::string16& key, + const base::string16& value, + const GURL& page_url) { ++#if 0 + // A quick check to reject obviously overbudget items to avoid + // the priming the cache. + if ((key.length() + value.length()) * sizeof(base::char16) > + kPerStorageAreaQuota) + return false; ++#endif + + PrimeIfNeeded(connection_id); + base::NullableString16 old_value; +diff --git a/content/renderer/dom_storage/local_storage_cached_area.cc b/content/renderer/dom_storage/local_storage_cached_area.cc +index ffa21c9200d0..0edbd6152292 100644 +--- a/content/renderer/dom_storage/local_storage_cached_area.cc ++++ b/content/renderer/dom_storage/local_storage_cached_area.cc +@@ -141,11 +141,13 @@ bool LocalStorageCachedArea::SetItem(const base::string16& key, + const base::string16& value, + const GURL& page_url, + const std::string& storage_area_id) { ++#if 0 + // A quick check to reject obviously overbudget items to avoid priming the + // cache. + if ((key.length() + value.length()) * sizeof(base::char16) > + kPerStorageAreaQuota) + return false; ++#endif + + EnsureLoaded(); + bool result = false; diff --git a/patches/common/chromium/dom_storage_map.patch b/patches/common/chromium/dom_storage_map.patch deleted file mode 100644 index afa9386add6d1..0000000000000 --- a/patches/common/chromium/dom_storage_map.patch +++ /dev/null @@ -1,23 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Cheng Zhao -Date: Thu, 20 Sep 2018 17:45:55 -0700 -Subject: dom_storage_map.patch - - -diff --git a/content/common/dom_storage/dom_storage_map.cc b/content/common/dom_storage/dom_storage_map.cc -index fd088fb170bead6452ded14016f21f0c29659e03..a9e4b3375e9b614ed1e8b737a30f4436adb12c37 100644 ---- a/content/common/dom_storage/dom_storage_map.cc -+++ b/content/common/dom_storage/dom_storage_map.cc -@@ -185,10 +185,12 @@ bool DOMStorageMap::SetItemInternal(MapType* map_type, - size_t new_item_size = size_in_storage(key, value); - size_t new_storage_used = storage_used_ - old_item_size + new_item_size; - -+#if 0 // Disable localStorage size limit for Electron. - // Only check quota if the size is increasing, this allows - // shrinking changes to pre-existing files that are over budget. - if (new_item_size > old_item_size && new_storage_used > quota_) - return false; -+#endif - - (*map_type)[key] = value; - ResetKeyIterator(); diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index d9d0d030f45c6..3362b9f6b23e9 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -938,6 +938,20 @@ describe('chromium feature', () => { }) describe('storage', () => { + describe('DOM storage quota override', () => { + ['localStorage', 'sessionStorage'].forEach((storageName) => { + it(`allows saving at least 50MiB in ${storageName}`, () => { + const storage = window[storageName] + const testKeyName = '_electronDOMStorageQuotaOverrideTest' + // 25 * 2^20 UTF-16 characters will require 50MiB + const arraySize = 25 * Math.pow(2, 20) + storage[testKeyName] = new Array(arraySize).fill('X').join('') + expect(storage[testKeyName]).to.have.lengthOf(arraySize) + delete storage[testKeyName] + }) + }) + }) + it('requesting persitent quota works', (done) => { navigator.webkitPersistentStorage.requestQuota(1024 * 1024, (grantedBytes) => { assert.strictEqual(grantedBytes, 1048576)