Last Comment Bug 463205 - It's possible to make SessionStore inject text data into the wrong document
: It's possible to make SessionStore inject text data into the wrong document
Status: RESOLVED FIXED
: [sg:high][fixed by bug 464620]
: verified1.8.1.19, verified1.9.0.5
Product: Firefox
Classification: Client Software
Component: Session Restore
: unspecified
: x86 Windows XP
: -- normal (vote)
: Firefox 3.1b2
Assigned To: Simon Bünzli
: session.restore
:
: 464620
:
  Show dependency treegraph
 
Reported: 2008-11-05 04:28 PST by moz_bug_r_a4
Modified: 2010-02-13 12:37 PST (History)
12 users (show)
dveditz: blocking1.9.0.5+
dveditz: wanted1.9.0.x+
dveditz: blocking1.8.1.19+
dveditz: wanted1.8.1.x+
asac: wanted1.8.0.x-
See Also:
Crash Signature:


Attachments
like so? (7.08 KB, patch)
2008-11-05 15:00 PST, Simon Bünzli
dietrich: review+
mbeltzner: approval1.9.1b2-
Details | Diff | Splinter Review
branch patch (1.22 KB, patch)
2008-11-12 03:34 PST, Simon Bünzli
dveditz: approval1.8.1.19+
dveditz: approval1.9.0.5+
Details | Diff | Splinter Review

Summon comment box

Description moz_bug_r_a4 2008-11-05 04:28:50 PST
SessionStore does not check whether a loaded document is an intended document
when restoring text data.  (On trunk, SessionStore checks a top-level
document's url, but does not check subframes.  On fx3/fx2, SessionStore does
not check at all.)  Thus, it's possible to make SessionStore inject text data
into the wrong document by loading a new document during restoration.
Comment 1 moz_bug_r_a4 2008-11-05 04:30:25 PST
Created attachment 346444 [details]
testcase 1

This tries to inject text data into http://htmledit.squarefree.com/
Comment 2 moz_bug_r_a4 2008-11-05 04:31:35 PST
Created attachment 346445 [details]
testcase 2

This tries to steal text data from
http://mxr.mozilla.org/mozilla-central/source/netwerk/testserver/docs/post.html?raw=1&ctype=text/html
Comment 3 moz_bug_r_a4 2008-11-05 05:31:07 PST
testcase 1 does not work in https: on fx3/fx2.
Comment 4 moz_bug_r_a4 2008-11-05 05:32:48 PST
Created attachment 346452 [details]
testcase 1 (+https)

This tries to inject text data into http://htmledit.squarefree.com/
Comment 5 Simon Bünzli 2008-11-05 15:00:08 PST
Created attachment 346550 [details] [review]
like so?
Comment 6 Daniel Veditz 2008-11-05 15:04:35 PST
Could be sg:moderate because of the session-restore requirement, but it's not hard to imagine a page using a known crasher to effectively force a tab reload so sg:high for now.
Comment 7 Dietrich Ayala (:dietrich) 2008-11-10 20:43:47 PST
Comment on attachment 346550 [details] [review]
like so?

r=me, looks to resolve this case. minor issue: is there a case where the loaded URI might differ from the string-serialized URI in a valid way? if so, maybe nsIURI.equals() would be better?
Comment 8 Simon Bünzli 2008-11-12 03:30:32 PST
Comment on attachment 346550 [details] [review]
like so?

Requesting approval for Beta 2 due to [sg:high].

(In reply to comment #7)
> is there a case where the loaded URI might differ from the string-
> serialized URI in a valid way?

Not AFAICT, as it is ourselves who restored that frame in the first place.
Comment 9 Simon Bünzli 2008-11-12 03:34:37 PST
Created attachment 347747 [details] [review]
branch patch
Comment 10 Dietrich Ayala (:dietrich) 2008-11-12 10:45:58 PST
Simon, is 1.8.1 branch affected? If so, can you make a patch for that as well?
Comment 11 Simon Bünzli 2008-11-12 11:49:35 PST
Comment on attachment 347747 [details] [review]
branch patch

This patch applies to the 1.8.1 branch as well.
Comment 12 Daniel Veditz 2008-11-13 10:18:51 PST
Comment on attachment 347747 [details] [review]
branch patch

approved for 1.8.1.19 and 1.9.0.5, a=dveditz for release-drivers
Comment 13 Mike Beltzner [:beltzner] 2008-11-14 14:50:21 PST
Comment on attachment 346550 [details] [review]
like so?

a1.9.1b2=beltzner
Comment 14 Simon Bünzli 2008-11-17 11:31:50 PST
There's a more comprehensive fix in bug 464620. We'll still want to land the tests from attachment #346550 [details] [review], though.
Comment 15 Mike Beltzner [:beltzner] 2008-11-19 08:08:33 PST
Comment on attachment 346550 [details] [review]
like so?

We're closing up for beta2 and this hasn't landed yet, so we'll hold off until after we branch.

Also: was this fixed on trunk by bug 464620 as indicated in the previous comment?
Comment 16 Simon Bünzli 2008-11-19 08:12:50 PST
This issue was indeed FIXED by bug 464620 on Trunk and both the 1.8.1 and 1.9 branches. The tests will land in bug 464620 as well.
Comment 17 Al Billings [:abillings] 2008-11-25 17:01:09 PST
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.

Note You need to log in before you can comment on or make changes to this bug.