Closed Bug 397524 Opened 17 years ago Closed 16 years ago

[FIX]Canvas security checks should use principals, not URIs

Categories

(Core :: Graphics: Canvas2D, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Fix (obsolete) — Splinter Review
Like the subject says.
Attachment #282294 - Flags: superreview?(jst)
Attachment #282294 - Flags: review?(vladimir)
Whoops, hit return to soon.  Yes, we can remove ComputeScaleFactor, given that all of our images are 24bpp now coming in from thebes.
Attachment #282294 - Flags: superreview?(jst) → superreview+
Comment on attachment 282294 [details] [diff] [review]
Fix

Requesting approval.  This is a pretty straightforward patch to improve some security checks.
Attachment #282294 - Flags: approval1.9?
Depends on: 389188
Off my plate until imagelib gets its security act together, apparently.  :(  See bug 389188.
Assignee: bzbarsky → nobody
Assignee: nobody → bzbarsky
Reed, see comment 3.
Assignee: bzbarsky → nobody
(In reply to comment #4)
> Reed, see comment 3.

Ah, sorry. I was fixing lots of bugs incorrectly assigned to nobody@, so sorry I didn't notice that.
This shows up on the search for "Unassigned M9 Blockers"
(http://tinyurl.com/2unjnx), but it is not marked blocking1.9. Should it be?
Flags: blocking1.9?
> This shows up on the search for "Unassigned M9 Blockers"

That search is just incorrect.
Blocks, but not M9.
Flags: blocking1.9? → blocking1.9+
Target Milestone: mozilla1.9 M9 → ---
Attachment #282294 - Flags: approval1.9? → approval1.9+
don't think we would block the release on this, but we'll happily take the fix!
Flags: blocking1.9+ → blocking1.9-
Assignee: nobody → bzbarsky
Attachment #282294 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
To test:

1)  Create a signed jar and put an image in it.
2)  Have a page outside the signed jar but loaded from the same host load the
    image.
3)  Have that page paint the image into a canvas.
4)  Try to get pixel data from the canvas.

Step 4 should throw, with this patch, and not throw without this patch.
Flags: in-testsuite?
In nsCanvasRenderingContext2D::CairoSurface there is a 'nsCOMPtr<nsIURI> uri;' which is no longer used.

And should we not also replace the URI with this GetPrinciple in:
mozilla/extensions/canvas3d/src/nsCanvasRenderingContextGL.cpp
as well? (also the 'GetURI' there seems to be of no use).

Re-open or new bug?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
New bug, please.
Status: REOPENED → RESOLVED
Closed: 17 years ago16 years ago
Resolution: --- → FIXED
> In nsCanvasRenderingContext2D::CairoSurface there is a 'nsCOMPtr<nsIURI> uri;'
> which is no longer used.

Looks like it was unused before this patch too.  Worth removing, but not reopening this bug.

> mozilla/extensions/canvas3d/src/nsCanvasRenderingContextGL.cpp

I asked vlad about it when I was working on this.  He said not to touch it because it was still so much in flux.  If we ever get close to shipping it anywhere, then yes we need to fix it.
this bug is in the fixing range for Bug 355126

is it what fixed it?
Yep.  The principal of an image is the post-redirect "where it really came from" principal.
In fact, checked in a test for this based on that.

Still need a test per comment 12.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: