355365 - Error: pnglets is not defined when visiting pnglets site

Status: RESOLVED WONTFIX

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

Description

[Martijn Wargers (dead)]

This is a spin-off from bug 355358, see bug 355358, comment 1:
"
I see multiple times (in the error console):

Error: pnglets is not defined
Source File: pnglets.line2
Line: 1
"
That used to be not happening.

This regressed between 2006-09-23 and 2006-09-24, I think it's a regression from bug 351370. 
If desired, I could try and make a minimal testcase.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah, the patch for bug 351370 would "break" this (in that now we deny access to all props always instead of only some and randomly)...  The question is whether we really care.

If we do, we need to change the principal we're giving the image javascript: loads, as Brendan keeps pushing for.

Comment 2

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

Yeah, I think we should add another one-off for images here, as we did for clicked links.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The question is whether it's a generally acceptable authoring practice that <img>s with javascript: SRC need to be filtered out.  If it is, then we can use the page principal.  If it's not, we can't.

Note also that IE7 either doesn't run such javascript: code at all or does it in some other script context, in my testing...

I'm going to post to m.d.security with some general thoughts on javascript:.

Comment 4

[Boris Zbarsky [:bzbarsky]]

See http://groups.google.com/group/mozilla.dev.security/browse_frm/thread/55e9d2c1a6781d94/ed16140c9c1a8e4c#ed16140c9c1a8e4c

Comment 5

[Martijn Wargers (dead)]

The branch patch from bug 351370 also made this regress on branch :( (see also bug 371633)

Comment 6

[Boris Zbarsky [:bzbarsky]]

The point is, this doesn't work in other UAs anyway (IE7 comes to mind).  Given that, I don't think we care so much about trying to fix this on branch, especially since we don't have a particularly safe way to do it...

Bug 369239 covers the work needed to be able to implement this safely.

Comment 7

[Boris Zbarsky [:bzbarsky]]

The above is my opinion, of course.  ;)

Comment 8

[bill]

Bug 371633 has been flagged as duplicate of this one.
May be its worth saying 371633 includes a new example: http://www.cs.ucl.ac.uk/staff/W.Langdon/image_javascript.html
which shows the problem exists for the simpler XBM image format as well as PNG.

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> Bug 371633 has been flagged as duplicate of this one.
> May be its worth saying 371633 includes a new example:
> http://www.cs.ucl.ac.uk/staff/W.Langdon/image_javascript.html
> which shows the problem exists for the simpler XBM image format as well as PNG.

The problem is independent of image type.

bz, is the best branch fix to back-port the null principals infrastructure and use it? Or could we use a sandbox type already implemented on the branch?

/be

Comment 10

[Boris Zbarsky [:bzbarsky]]

Brendan, the only possible branch fix is to propagate the loader principal through imagelib, which requires imagelib API changes.

In more detail, right now we're not propagating the principal, so we have no idea who's doing the load.  We used to assume that that made it ok to run against the page context with a null principal like thing, but that gave us bug 351370.  So now if we don't know who's doing the load we run it in a sandbox.  The sites in question depend on the script in the image src touching the page's script context.  So they will absolutely not work while that script is sandboxed -- the only way for them to work correctly is for the script to run against the page context with the page's principals.

Note that the only reason this used to work on branch at all is due to bug 350433 (because we used to run against the page context but NOT with the page's principals, so the fact that the script could touch the things it was touching is actually a security hole).

Comment 11

[Brendan Eich [:brendan]]

Then let's make a MOZILLA_1_8_BRANCH API to get the owner propagated to the JS URL channel. I know IE does whatever it does, but we need to take compatibility with our own (going back to Netscape 2) functionality pretty seriously. Bugs or not, we have supported something like working image generation via javascript: img src for a long time, most of the time.

What would the branch API look like? Single method IDL-defined interface to QI for in the branch imglib code?

/be

Comment 12

[Boris Zbarsky [:bzbarsky]]

Dunno what API would be acceptable to Stuart.  He's the man to ask though, not I!  ;)

Comment 13

[Daniel Veditz [:dveditz]]

With an owner and a fix we can take a look, but not going to block a branch security release.

Comment 14

[Martijn Wargers (dead)]

So can this be marked WONTFIX then?

Comment 15

[Martijn Wargers (dead)]

Should this be marked WONTFIX?