From: https://bugs.webkit.org/show_bug.cgi?id=32369#c3 I think sessionStorage should be disabled in sandboxed iframes. sandboxed.html: <script>sessionStorage.x='busted';</script> sandboxer.html: <script>setInterval(function(){document.title=sessionStorage.x},500);</script><iframe sandbox="allow-scripts" src="sandboxed.html"></iframe>
the previous code will set document.title to "busted", if the page is saving sensitive information in there, then it may introduce security problems.
According to http://dev.w3.org/html5/webstorage/#the-sessionstorage-attribute, we should have one per origin.
a sandboxed iframe is considered to have the same origin, or a new "unique" origin? from http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html "When the attribute is set, the content is treated as being from a unique origin"
We're interested in the case where it has a unique origin. The "correct" behavior is probably to create a new storage area for the unique origin. I'm working up a patch that throws a security exception because that matches what we're doing for localStorage.
is it useful to have sessionStorage on a window that has a unique origin every time is created?
oh nvm, I just read your comment. a sec error is cool
Here's the patch that turned this on: http://trac.webkit.org/changeset/56002
Created attachment 55155 [details] Patch
Attachment 55155 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2223001
Created attachment 55170 [details] Patch
Comment on attachment 55170 [details] Patch WebCore/storage/StorageEventDispatcher.cpp:60 + if (!ec) Could/should this be an early return instead of further indent to the line-that-never-wrapped? WebCore/storage/StorageEventDispatcher.cpp:61 + frames[i]->document()->enqueueEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, sourceFrame->document()->url(), storage)); I'm rather anti-wrap, but I think even I might have used a local variable here. WebKit/chromium/src/StorageAreaProxy.cpp:130 + Storage* storage = frames[i]->domWindow()->sessionStorage(ec); Why not just check storage != NULL? Seems OK, even if the EC handling is slightly inelegant.
Comment on attachment 55170 [details] Patch Adam should see my comments first.
Comment on attachment 55170 [details] Patch I think its ok for now. I copied the style from localStorage below. If we want to change it, we should change both at once.
Comment on attachment 55170 [details] Patch Clearing flags on attachment: 55170 Committed r58873: <http://trac.webkit.org/changeset/58873>
All reviewed patches have been landed. Closing bug.