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

RESOLVED FIXED in Firefox 3.1b2

Status

()

Firefox
Session Restore
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: moz_bug_r_a4, Assigned: Simon Bünzli)

Tracking

({verified1.8.1.19, verified1.9.0.5})

unspecified
Firefox 3.1b2
x86
Windows XP
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][fixed by bug 464620])

Attachments

(2 obsolete attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
Created attachment 346444 [details]
testcase 1

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

Comment 3

10 years ago
testcase 1 does not work in https: on fx3/fx2.
(Reporter)

Comment 4

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

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

Comment 5

10 years ago
Created attachment 346550 [details] [diff] [review]
like so?
Attachment #346550 - Flags: review?(dietrich)
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.
Assignee: nobody → zeniko
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 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?
Attachment #346550 - Flags: review?(dietrich) → review+
Whiteboard: [sg:high][needs r dietrich] → [sg:high]
(Assignee)

Comment 8

10 years ago
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.
Attachment #346550 - Flags: approval1.9.1b2?
(Assignee)

Comment 9

10 years ago
Created attachment 347747 [details] [diff] [review]
branch patch
Attachment #347747 - Flags: approval1.9.0.5?
Simon, is 1.8.1 branch affected? If so, can you make a patch for that as well?
Whiteboard: [sg:high] → [sg:high][has review][needs approval]
Target Milestone: --- → Firefox 3.1b2
(Assignee)

Comment 11

10 years ago
Comment on attachment 347747 [details] [diff] [review]
branch patch

This patch applies to the 1.8.1 branch as well.
Attachment #347747 - Flags: approval1.8.1.18?
(Assignee)

Updated

10 years ago
Attachment #347747 - Flags: approval1.8.1.18? → approval1.8.1.19?
Attachment #347747 - Flags: approval1.9.0.5?
Attachment #347747 - Flags: approval1.9.0.5+
Attachment #347747 - Flags: approval1.8.1.19?
Attachment #347747 - Flags: approval1.8.1.19+
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
Attachment #346550 - Flags: approval1.9.1b2? → approval1.9.1b2+
(Assignee)

Comment 14

10 years ago
There's a more comprehensive fix in bug 464620. We'll still want to land the tests from attachment #346550 [details] [diff] [review], though.
Whiteboard: [sg:high][has review][needs approval] → [sg:high][has review][to be fixed in bug 464620]
Whiteboard: [sg:high][has review][to be fixed in bug 464620] → [sg:high][has review][has approval]
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?
Attachment #346550 - Flags: approval1.9.1b2+ → approval1.9.1b2-
(Assignee)

Comment 16

10 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [sg:high][has review][has approval] → [sg:high][fixed by bug 464620]
(Assignee)

Updated

10 years ago
Attachment #346550 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Attachment #347747 - Attachment is obsolete: true
Keywords: fixed1.8.1.19, fixed1.9.0.5
Depends on: 464620
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.
Keywords: fixed1.8.1.19, fixed1.9.0.5 → verified1.8.1.19, verified1.9.0.5

Updated

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