354127 - stealing pictures using canvas toDataURL

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[georgi - hopefully not receiving bugspam]

stealing pictures

using two canvases, fillRect, drawImage and toDataURL it is possible to get
the content of arbitrary accessible images.

testcase to follow.

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: stealing pictures

Comment 2

[Daniel Veditz [:dveditz]]

I guess if there were a sensitive image inside a firewall the user could see but the attacker couldn't reach directly (but somehow guessed the name of--disgruntled insider?) then this could be used to steal those insider images. Wouldn't the real fix be to block "public" pages from including content from "private" URLs?

But you could also use the :visited trick (bug 147777) to see if the victim might be logged on somewhere interesting (or just guess) and steal images that might be plots of sensitive data. Textual data is protected by the same-origin policy (can't reach into a frame with that data, can't use XMLHttpRequest to get it), images have up to now been opaque blobs.

The what-wg spec apparently considered this case, saying in 6.1:
  "Security: To prevent information leakage, the toDataURL() and
  getImageData() methods should raise a security exception if the
  canvas ever had images painted on it that originate from a domain
  other than the domain of the script that painted the images onto
  the canvas."

It looks like canvas attempts to do this. data URLs can't be gotten off the first canvas, only after drawImage() is used to create the second canvas. The problem appears to be that CairoSurfaceFromElement() returns a null URI from a canvas element (makes sense) but then DoDrawImageSecurityCheck() requires that auri not be null to do anything. I think we just switch the order of checks there to fix this.

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: Reverse order in DoDrawImageSecurityCheck

With this patch the testcase throws a security exception

Comment 4

[Daniel Veditz [:dveditz]]

Probably too late for FF2 initial, but worth trying since this is a new feature and may get scrutiny out of the gate.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 240035 [details] [diff] [review]
Reverse order in DoDrawImageSecurityCheck

r+sr=me

Comment 6

[Mike Schroepfer]

Safe enough for RC2?  If so nom the patch..

Comment 7

[Mike Schroepfer]

Comment on attachment 240035 [details] [diff] [review]
Reverse order in DoDrawImageSecurityCheck

Approved for RC2.  Please land and mark Fixed1.8.1

Comment 8

[Daniel Veditz [:dveditz]]

Fix checked into trunk and 1.8 branch

Comment 9

[georgi - hopefully not receiving bugspam]

seems fixed on trunk without known regression.