SessionStore does not correctly restore text data when subframes are involved

VERIFIED FIXED in Firefox 3.1b2

Status

()

VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: moz_bug_r_a4, Assigned: zeniko)

Tracking

({verified1.8.1.19, verified1.9.0.5})

unspecified
Firefox 3.1b2
verified1.8.1.19, verified1.9.0.5
Points:
---
Bug Flags:
blocking1.9.0.5 +
wanted1.9.0.x +
blocking1.8.1.19 +
wanted1.8.1.x +
wanted1.8.0.x -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:high])

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
This is fx3/fx2 only.

There are two problems.

1.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/sessionstore/src/nsSessionStore.js&rev=1.102&mark=1081-1088#1081
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/sessionstore/src/nsSessionStore.js&rev=1.102&mark=1730-1734#1730

When saving text data, "0|1|2|" is used as the id prefix for frames[0][1][2].
But when restoring text data, "2|1|0|" is used as the id prefix for
frames[0][1][2].

2.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/sessionstore/src/nsSessionStore.js&rev=1.102&mark=1703#1701

SessionStore tries to inject text data of a top-level document into subframes.
(Reporter)

Comment 1

10 years ago
Created attachment 346446 [details]
testcase 1

This demonstrates the problem 1.
(Reporter)

Comment 2

10 years ago
Created attachment 346447 [details]
testcase 2

This demonstrates the problem 2.
(Reporter)

Comment 3

10 years ago
Created attachment 346448 [details]
testcase 3 - XSS using the problem 1

This tries to inject text data into http://htmledit.squarefree.com/
(Reporter)

Comment 4

10 years ago
Created attachment 346449 [details]
testcase 4 - XSS using the problem 2

This tries to inject text data into http://htmledit.squarefree.com/
(Reporter)

Comment 5

10 years ago
testcase 1 and 3 do not work in https:.
(Reporter)

Comment 6

10 years ago
Created attachment 346454 [details]
testcase 1 (+https)

This demonstrates the problem 1.
(Reporter)

Comment 7

10 years ago
Created attachment 346456 [details]
testcase 3 (+https) - XSS using the problem 1

This tries to inject text data into http://htmledit.squarefree.com/
Flags: blocking1.9.0.5?
Flags: blocking1.8.1.19?
(Assignee)

Comment 8

10 years ago
Created attachment 346519 [details] [diff] [review]
trunk patch (for legacy routines) and tests
Assignee: nobody → zeniko
Status: NEW → ASSIGNED
Attachment #346519 - Flags: review?
(Assignee)

Comment 9

10 years ago
Created attachment 346522 [details] [diff] [review]
trunk patch (for legacy routines) and tests
Attachment #346519 - Attachment is obsolete: true
Attachment #346522 - Flags: review?(dietrich)
Attachment #346519 - Flags: review?
(Assignee)

Comment 10

10 years ago
Created attachment 346523 [details] [diff] [review]
branch patch
Attachment #346523 - Flags: review?(dietrich)
Whiteboard: [sg:high]
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.5?
Flags: blocking1.9.0.5+
Flags: blocking1.8.1.19?
Flags: blocking1.8.1.19+
Whiteboard: [sg:high] → [sg:high][needs r dietrich]
Comment on attachment 346522 [details] [diff] [review]
trunk patch (for legacy routines) and tests

r=me
Attachment #346522 - Flags: review?(dietrich) → review+
Comment on attachment 346523 [details] [diff] [review]
branch patch

>Index: browser/components/sessionstore/src/nsSessionStore.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v
>retrieving revision 1.102
>diff -u -8 -d -p -r1.102 nsSessionStore.js
>--- browser/components/sessionstore/src/nsSessionStore.js	24 Oct 2008 20:33:25 -0000	1.102
>+++ browser/components/sessionstore/src/nsSessionStore.js	5 Nov 2008 21:06:57 -0000
>@@ -1054,16 +1054,20 @@ SessionStoreService.prototype = {
>   _saveTextData: function sss_saveTextData(aPanel, aTextarea) {
>     var id = aTextarea.id ? "#" + aTextarea.id :
>                             aTextarea.name;
>     if (!id
>       || !(aTextarea instanceof Ci.nsIDOMHTMLTextAreaElement 
>       || aTextarea instanceof Ci.nsIDOMHTMLInputElement && aTextarea.type != "password")) {
>       return false; // nothing to save
>     }
>+    if (/^(?:\d+\|)+/.test(id)) {
>+      // text could be restored into a subframe, so skip it (see bug 463206)
>+      return false;
>+    }
>     
>     if (!aPanel.__SS_text) {
>       aPanel.__SS_text = [];
>       aPanel.__SS_text._refs = [];
>     }
>     
>     // get the index of the reference to the text element
>     var ix = aPanel.__SS_text._refs.indexOf(aTextarea);
>@@ -1695,17 +1699,17 @@ SessionStoreService.prototype = {
>     // wait for the top frame to be loaded completely
>     if (!aEvent || !aEvent.originalTarget || !aEvent.originalTarget.defaultView || aEvent.originalTarget.defaultView != aEvent.originalTarget.defaultView.top) {
>       return;
>     }
>     
>     var textArray = this.__SS_restore_text ? this.__SS_restore_text.split(" ") : [];
>     function restoreTextData(aContent, aPrefix) {
>       textArray.forEach(function(aEntry) {
>-        if (/^((?:\d+\|)*)(#?)([^\s=]+)=(.*)$/.test(aEntry) && (!RegExp.$1 || RegExp.$1 == aPrefix)) {
>+        if (/^((?:\d+\|)*)(#?)([^\s=]+)=(.*)$/.test(aEntry) && RegExp.$1 == aPrefix) {
>           var document = aContent.document;
>           var node = RegExp.$2 ? document.getElementById(RegExp.$3) : document.getElementsByName(RegExp.$3)[0] || null;
>           if (node && "value" in node) {
>             node.value = decodeURI(RegExp.$4);
>             
>             var event = document.createEvent("UIEvents");
>             event.initUIEvent("input", true, true, aContent, 0);
>             node.dispatchEvent(event);
>@@ -1724,17 +1728,17 @@ SessionStoreService.prototype = {
>                 }
>               }, 0, aData.innerHTML);
>       }
>       if (aData.scroll && /(\d+),(\d+)/.test(aData.scroll)) {
>         aContent.scrollTo(RegExp.$1, RegExp.$2);
>       }
>       for (var i = 0; i < aContent.frames.length; i++) {
>         if (aData.children && aData.children[i]) {
>-          restoreTextDataAndScrolling(aContent.frames[i], aData.children[i], i + "|" + aPrefix);
>+          restoreTextDataAndScrolling(aContent.frames[i], aData.children[i], aPrefix + i + "|");
>         }
>       }
>     }
>     
>     var content = aEvent.originalTarget.defaultView;
>     if (this.currentURI.spec == "about:config") {
>       // unwrap the document for about:config because otherwise the properties
>       // of the XBL bindings - as the textbox - aren't accessible (see bug 350718)
Attachment #346523 - Flags: review?(dietrich) → review+
Whiteboard: [sg:high][needs r dietrich] → [sg:high]
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 346522 [details] [diff] [review]
trunk patch (for legacy routines) and tests

requesting a= for b2 due to the sg:high.
Attachment #346522 - Flags: approval1.9.1b2?
Attachment #346522 - Flags: approval1.9.1b2? → approval1.9.1b2+
Comment on attachment 346522 [details] [diff] [review]
trunk patch (for legacy routines) and tests

a=beltzner, sound rationale
Whiteboard: [sg:high] → [sg:high][has review][has approval]
Target Milestone: --- → Firefox 3.1b2
Simon, is 1.8.1 branch affected? If so, can you make a patch for that as well?
(Assignee)

Comment 16

10 years ago
Created attachment 347822 [details] [diff] [review]
branch patch (1.8.1)
Attachment #347822 - Flags: approval1.8.1.18?
(Assignee)

Updated

10 years ago
Attachment #347822 - Flags: approval1.8.1.18? → approval1.8.1.19?
Attachment #347822 - Flags: approval1.8.1.19? → approval1.8.1.19+
Comment on attachment 347822 [details] [diff] [review]
branch patch (1.8.1)

Approved for 1.8.1.18, a=dveditz for release-drivers
Comment on attachment 346523 [details] [diff] [review]
branch patch

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #346523 - Flags: approval1.9.0.5+
Trunk patch pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/5e1a889aad0e
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Can we get this landed in 1.9.0 and 1.8.1? I don't think this is part of the roll up patch...
(Assignee)

Comment 21

10 years ago
Created attachment 349067 [details] [diff] [review]
[checked in] 1.9 branch patch

That's the only bit that I embarrassingly forgot to include in the roll-up patch.
Attachment #346523 - Attachment is obsolete: true
(Assignee)

Comment 22

10 years ago
Created attachment 349068 [details] [diff] [review]
[checked in] 1.8.1 branch patch
Attachment #347822 - Attachment is obsolete: true
(Assignee)

Comment 23

10 years ago
I'll need somebody to land these patches for me, though. Thanks in advance!
Comment on attachment 349067 [details] [diff] [review]
[checked in] 1.9 branch patch

Checked in on branch. (looks like the rest of this previously landed as bug 464620)

Checking in browser/components/sessionstore/src/nsSessionStore.js;
  new revision: 1.107; previous revision: 1.106
Attachment #349067 - Attachment description: 1.9 branch patch → [checked in] 1.9 branch patch
Keywords: fixed1.9.0.5
Comment on attachment 349068 [details] [diff] [review]
[checked in] 1.8.1 branch patch

Checked in on 1.8 branch. (looks like the rest of this previously landed as bug
464620)

Checking in browser/components/sessionstore/src/nsSessionStore.js;
  new revision: 1.5.2.54; previous revision: 1.5.2.53
Attachment #349068 - Attachment description: 1.8.1 branch patch → [checked in] 1.8.1 branch patch
Keywords: checkin-needed → fixed1.8.1.19
Whiteboard: [sg:high][has review][has approval] → [sg:high]
Verified for 1.8.1.19 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.19pre) Gecko/2008112503 BonEcho/2.0.0.19pre. 

Verified for 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008112505 GranParadiso/3.0.5pre. 

Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081123 Minefield/3.1b2pre.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.19, fixed1.9.0.5 → verified1.8.1.19, verified1.9.0.5

Comment 27

10 years ago
no issue on 1.8.0.
Flags: wanted1.8.0.x-
Group: core-security
You need to log in before you can comment on or make changes to this bug.