463205 - It's possible to make SessionStore inject text data into the wrong document

Status: RESOLVED FIXED

Description

[moz_bug_r_a4]

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]

Created attachment 346444 [details]
testcase 1

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

Comment 2

[moz_bug_r_a4]

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]

testcase 1 does not work in https: on fx3/fx2.

Comment 4

[moz_bug_r_a4]

Created attachment 346452 [details]
testcase 1 (+https)

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

Comment 5

[Simon Bünzli]

Created attachment 346550 [details] [diff] [review]
like so?

Comment 6

[Daniel Veditz [:dveditz]]

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)]

Comment on attachment 346550 [details] [diff] [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]

Comment on attachment 346550 [details] [diff] [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]

Created attachment 347747 [details] [diff] [review]
branch patch

Comment 10

[Dietrich Ayala (:dietrich)]

Simon, is 1.8.1 branch affected? If so, can you make a patch for that as well?

Comment 11

[Simon Bünzli]

Comment on attachment 347747 [details] [diff] [review]
branch patch

This patch applies to the 1.8.1 branch as well.

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 347747 [details] [diff] [review]
branch patch

approved for 1.8.1.19 and 1.9.0.5, a=dveditz for release-drivers

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 346550 [details] [diff] [review]
like so?

a1.9.1b2=beltzner

Comment 14

[Simon Bünzli]

There's a more comprehensive fix in bug 464620. We'll still want to land the tests from attachment #346550 [details] [diff] [review], though.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 346550 [details] [diff] [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]

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]]

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.