Closed
Bug 614116
(CVE-2011-0052)
Opened 14 years ago
Closed 14 years ago
Insecure sites may modify existing secure items in globalStorage when in PB or SO-cookies mode
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
People
(Reporter: mayhemer, Assigned: ehsan.akhgari)
References
Details
(Keywords: privacy, regression, Whiteboard: [sg:low])
Attachments
(1 file, 2 obsolete files)
9.20 KB,
patch
|
mayhemer
:
review+
dveditz
:
approval1.9.2.14+
dveditz
:
approval1.9.1.17+
|
Details | Diff | Splinter Review |
Regression from bug 501423. Before an non-secure page modifies an item in globalStorage, we first have to check the item is not present or is not secure (not stored by an https page). This is simple to fix by adding a check to nsDOMStorage::SetItem. SetDBValue just updates w/o any checks, expecting those are made by callers. Love to have a test for this as well.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Reporter | ||
Comment 1•14 years ago
|
||
This is valid only in session-only cookies or PB mode. The check is actually missing in nsDOMStorageMemoryDB::SetKey. nsDOMStorage::SetDBValue is not responsible for secure flag checking. nsDOMStoragePersistentDB::SetKey (called from SetDBValue) does it for persistent storages. This can potentially be used to detect PB or SO mode. Not sure if this necessarily needs to be a security bug any more.
Summary: Insecure sites may modify existing secure items in globalStorage → Insecure sites may modify existing secure items in globalStorage when in PB or SO-cookies mode
Updated•14 years ago
|
blocking2.0: ? → betaN+
Whiteboard: [sg:moderate]
Assignee | ||
Comment 2•14 years ago
|
||
If I read comment 1 correctly, our only concern is that the previously stored item in the memory DB is not from a secure site when nsDOMStorageMemoryDB::SetKey is called for a non-secure site, right?
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Updated•14 years ago
|
Whiteboard: [sg:moderate] → [sg:moderate][has patch][needs review mayhemer]
Reporter | ||
Comment 4•14 years ago
|
||
Comment on attachment 493605 [details] [diff] [review] Patch (v1) That's exactly what we need, thanks. Few details: - please move the test to dom/tests/mochitest/globalstorage (there will be soon this directory) - if you are willing to simplify the test a bit: there is a framework to write inter origin tests, see test_localStorageOriginsSchemaDiffs.html + frameMasterNotEqual.html + frameSlaveNotEqual.html, the test is using interOriginTest2.js and frames are using interOriginFrame.js ; it makes the tests much more readable and easier to review r=honzab with the first change and optionally the second one as well
Attachment #493605 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Comment 5•14 years ago
|
||
Moved the test, but otherwise left it as it was.
Attachment #493605 -
Attachment is obsolete: true
Assignee | ||
Comment 6•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/1f53f85ddfb5
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [sg:moderate][has patch][needs review mayhemer] → [sg:moderate]
Target Milestone: --- → mozilla2.0b8
Reporter | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > Do we also need this for branches? We do, this bug is present since 1.9.1. Introduced in bug 487695.
Assignee | ||
Comment 9•14 years ago
|
||
This broke a XUL file test in editor/. It turns out that calling pm.removeAll will remove the permission for allowXULXBL, which makes us not able to load the XUL file for that test. I have a patch to fix that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 10•14 years ago
|
||
Honza, can you please review the change?
Attachment #493730 -
Attachment is obsolete: true
Attachment #493792 -
Flags: review?(honzab.moz)
Reporter | ||
Comment 11•14 years ago
|
||
Comment on attachment 493792 [details] [diff] [review] Patch (v2) r=honzab.
Attachment #493792 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Updated•14 years ago
|
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Assignee | ||
Updated•14 years ago
|
Attachment #493792 -
Flags: approval1.9.2.14?
Attachment #493792 -
Flags: approval1.9.1.17?
Updated•14 years ago
|
Blocks: 487695
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
Comment 13•14 years ago
|
||
Comment on attachment 493792 [details] [diff] [review] Patch (v2) Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
Attachment #493792 -
Flags: approval1.9.2.14?
Attachment #493792 -
Flags: approval1.9.2.14+
Attachment #493792 -
Flags: approval1.9.1.17?
Attachment #493792 -
Flags: approval1.9.1.17+
Assignee | ||
Comment 14•14 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a63fd7a33fc7 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/340d74cbb09f
Comment 15•14 years ago
|
||
The test for this bug has been failing continuously (with a timeout) on the branches since it landed there. See bug 616219.
Comment 16•14 years ago
|
||
sg:moderate overstates the problem. You can't read secure data, only write, and only when the potential victim is in session-only/private mode. It's not hard to imagine scenarios where there's some website set up to do off-line processing and maybe has terrible things to overwrite if the MITM knew you were on that site, but in practice I can't think of any real cases.
Keywords: privacy
Whiteboard: [sg:moderate] → [sg:low]
Updated•14 years ago
|
Alias: CVE-2011-0052
Updated•14 years ago
|
Group: core-security
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•