377092 - Expose a way to set the originating principal of a javascript: URI on that URI

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Needed to fix javascript:-uri images in the short term.  Doing this should let pnglets work, perhaps.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Idea: do this by flagging the source principal of the javascript: URI on the URI (e.g. in nsContentUtils::LoadImage) and the channel using that if it has no owner itself.  That doesn't have the problems that flagging the execution policy on the URI does, and the source principal doesn't depend on whether the URI gets cloned, etc.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Er, probably won't fix pnglets.  But would fix anything that generates the image on the fly without reference to the web page, since we'd run the javascript: in a sandbox.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note: when we fix this, retest bug 363597 testcase 1 to make sure all is OK.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Need bug 310165 to be fixed first so we know what the loading principal is...

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm a little confused about what this bug is about. What does "on that URI" refer to in the summary?

Doesn't pinglets that doesn't reference the containing page already work? I.e. don't we execute javascript uris for images in a sandbox?

Either way, this doesn't really seem like a blocker, since however cool pinglets are, i've never heard of them being used on actual sites other than as demos.

Might be wanted-1.9, but I'd like to better understand what the bug is about.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> What does "on that URI" refer to in the summary?

Have a way to indicate on a particular nsIURI object for a javascript: URI what principal the creator of the URI has.

> Doesn't pinglets that doesn't reference the containing page already work?

Right now, no.  We don't execute javascript: URIs at all if we don't know the origin principal, because we need that principal to do the "is javascript enabled?" check.  I don't think we want to change that to assuming script is enabled.

Since we currently only pass principals along with channels, and imagelib doesn't expose channels, that means no javascript: in any image loads right now.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix, plus tests

biesi, would you look over the URI code?  jst, could you review the rest?

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

+        nsCOMPtr<nsIScriptURI> scriptURI = do_QueryInterface(mURI);
+        if (scriptURI) {

won't you always have a scriptURI here? (i.e. shouldn't that if be an assertion?)

+    *aClassID = (nsCID*) nsMemory::Alloc(sizeof(nsCID));

NS_Alloc

--

how does this interact with image cache? that is... could one site get an image added to the cache with its principal, and the other site could use the same URL (but a different principal) and access that?
or does the error from Equals deal with that?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

is there a particular reason why you pass on some setters to the inner URI? they fail anyway, but it might be clearer to make them return an error too?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> won't you always have a scriptURI here? 

Probably, yeah.  I suppose I could make this assert.

> NS_Alloc

Sure.

> or does the error from Equals deal with that?

Oops.  I need to actually implement Equals (and have it compare principals too).  Good catch!  I was going to do that, and forgot...

I would hope that even if the image cache caches this (which I seem to recall it didn't) that would help.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 272130 [details] [diff] [review]
Fix, plus tests

ok, afaict imagelib does cache this

Comment 12

[Boris Zbarsky [:bzbarsky]]

biesi points out that there's really nothing I can do on the javascript: end to prevent imagelib from caching javascript: images across different documents.  Unless I make javascript: implement nsICachingChannel, which would screw up other consumers.

Over to pav to get us some sort of imagelib arch where security stuff can possibly work.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Note in particular that imagelib does string compares instead of using nsIURI::Equals for cache purposes.  Which is really rather broken.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.