Last Comment Bug 325005 - Documents parsed as data load subframes and objects
: Documents parsed as data load subframes and objects
Status: RESOLVED FIXED
[sg:moderate][rft-dl]
: fixed1.8.1, verified1.8.0.2
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
: Hixie (not reading bugmail)
Mentors:
javascript:void(new DOMParser().parse...
Depends on:
Blocks: 325006
  Show dependency treegraph
 
Reported: 2006-01-27 20:49 PST by Boris Zbarsky [:bz] (Out June 25-July 6)
Modified: 2008-01-30 17:59 PST (History)
13 users (show)
dveditz: blocking1.7.14?
dveditz: blocking‑aviary1.0.9?
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.79 KB, patch)
2006-01-29 12:50 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review
Same as diff -w (2.05 KB, patch)
2006-01-29 12:51 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
jonas: review+
jst: superreview+
jst: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-27 20:49:27 PST
See URL in the URL field.  Should that alert a 1?  Or a 0?

We block loading of images, stylesheets, and scripts in a document loaded as data.  Should we also block loading of subdocuments and objects?  That is, perhaps we should remove the type check at http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDataDocumentContentPolicy.cpp#56 ?

Note that this would affect documents loaded via XMLDocument.load, XMLHttpRequest, and DOMParser.  Do we need to differentiate between these cases in any way here?
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-27 20:57:48 PST
Note that loading subframes means that script can execute inside an XMLHttpRequest or DOMParser document, like so:

javascript:void(new DOMParser().parseFromString("<html xmlns='http://www.w3.org/1999/xhtml'><iframe src='data:text/html,&lt;script&gt;alert(1);&lt;/script&gt;'/></html>", "text/xml"))

I'm not sure whether this is a problem, since I'm not sure that this script has any access to the caller document.  But this is still a little troubling.

At the very least, I would think that if we _do_ load subframes in a data document we should load them as data, right?
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-27 20:59:43 PST
Even simpler:

javascript:void(new DOMParser().parseFromString("<html xmlns='http://www.w3.org/1999/xhtml'><iframe src='javascript:alert(1)'/></html>", "text/xml"))

Comment 3 Jesse Ruderman 2006-01-27 21:34:51 PST
The script runs with the privileges of the page that used DOMParser, so it can make requests to that site, read cookies for that site, etc.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-27 22:24:37 PST
OK, that seems like a problem.  Sounds to me like we should just block all loads, not just some types.  Any objections?

Also, do we need this fixed on branches?  Doing that on 1.7 could be interesting... :(
Comment 5 Jonas Sicking (:sicking) PTO Until July 5th 2006-01-28 01:56:48 PST
Seeing that we have a same-origin policy in place, I'm not sure what the attack is here?

That said, it's of doubtful value, and probably unexpected for many users, to load external document in iframes and such.
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-29 12:50:38 PST
Created attachment 210059 [details] [diff] [review]
Patch
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-29 12:51:17 PST
Created attachment 210060 [details] [diff] [review]
Same as diff -w
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2006-01-30 15:39:10 PST
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

sr=jst
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-30 17:48:06 PST
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

We should probably take this on the 1.8 branch.  Jesse, do we need a fix for this on the 1.7 branch?
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-01-30 17:48:35 PST
Fixed on trunk.
Comment 11 Daniel Veditz [:dveditz] 2006-02-14 15:54:10 PST
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

approved for 1.8.0 branch, a=dveditz for drivers
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-02-14 18:41:42 PST
Fixed for 1.8.0.2.
Comment 13 Daniel Veditz [:dveditz] 2006-02-15 08:33:40 PST
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

I think we want this on the older branches too.
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-02-15 11:27:59 PST
That patch doesn't apply to the older branches, since they have no nsDataDocumentContentPolicy.  We could introduce that on the branches, but that will take some work, since the API is not the same (not the same between aviary and 1.7 branches, and not the same between either and trunk).

I suppose I can do that if desired.  :(
Comment 15 Jay Patel [:jay] 2006-03-08 17:11:13 PST
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2, bz's simple testcase in comment #2. 

Nothing happens... no alert, nothing in jsc, which is expected, right?  We simply reject the doc load and the js inside the iframe?
Comment 16 Jonas Sicking (:sicking) PTO Until July 5th 2006-03-08 18:15:51 PST
Right
Comment 17 [On PTO until 6/29] 2008-01-30 17:59:28 PST
Per Dveditz's request, I checked to see that this was still fixed for 1.8.1.12. It is.

Checked with  Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.12) Gecko/2008013015 Firefox/2.0.0.12

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