624763 - View selection source causes event handlers in document to fire

Status: RESOLVED FIXED

(Toolkit :: View Source, defect, P2)

Description

[Brandon Sterne (:bsterne)]

Attachment: select the image and "View Selection Source"

I'm not sure if Networking is the right component or if this is even a bug we'd fix, but I wanted to file it anyway to investigate.

When you have event-handlers on DOM elements like the one in the testcase, e.g.
<img src="http://www.mozilla.com/favicon.ico" onload="alert('load')">

Selecting the content and viewing selection source causes the event handlers to fire.  Doing a normal view source does not trigger the event handlers.  Is this just a side effect of having to reparse the DOM tree to figure out which portion is selected?  Is it an effect that we should try to prevent?

Comment 2

[Brandon Sterne (:bsterne)]

Adding the original reporter and hiding while we determine if this is a serious issue.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Is this just a side effect of having to reparse the DOM tree

What the heck does it mean to "reparse the DOM tree"?  ;)

This isn't a core issue.  viewPartialSourceForSelection in viewPartialSource.js has this gem:

123 // clone the fragment of interest and reset everything to be relative to it
124 // note: it is with the clone that we operate/munge from now on
125 ancestorContainer = ancestorContainer.cloneNode(true);

Cloning an image node will start the image load on the clone, of course.

The right way to do this is to have a data document (e.g. created with createDocument) and _import_ the fragment into that.  That way the normal data document content policy will prevent network access from elements in the clone (and event handlers on them couldn't run anyway, since a data document has no scripting environment).

Comment 4

[Daniel Veditz [:dveditz]]

Attachment: testcase altering the document

A Boris indicated, the event handler is run in the original document and thus can't do anything it hadn't already been able to do to you. This is a bug, but not a security concern.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #3)
> (and event handlers on them couldn't run anyway, since a data document has no
> scripting environment).
This is not quite true. Data documents do have script handling object, if
the document, which' implementation was used for createDocument has as
script handling object. But onfoo attributes don't map to event listeners on
data documents.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Unfortunately, a document created via document.implementation is not marked as loaded as data, and has working inline event handlers... so that approach doesn't work.

We could really use a way to say "clone this DOM bit as data"...

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Boris Zbarsky (:bz) from comment #6)
> Unfortunately, a document created via document.implementation is not marked
> as loaded as data

Really? I thought they were marked as data documents. Has something regressed that.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

As far as I see, nsContentUtils::CreateDocument does create a data document, and document.implementation uses that.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  I should have looked more carefully at _which_ document was the ownerDocument when the security check passed.  I just needed to make more changes to the view-source code.

Fix coming up.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: When cloning the fragment we'll munge, clone it into a data document.

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1cd65ceeb47

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b1cd65ceeb47