Last Comment Bug 346927 - drawImage corrupts transparent 24-bit PNGs with 1-bit-convertible alpha
: drawImage corrupts transparent 24-bit PNGs with 1-bit-convertible alpha
Status: RESOLVED FIXED
[sg:low]
: privacy, testcase, verified1.8.0.9, verified1.8.1.1
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: 1.8 Branch
: x86 Linux
-- normal (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail))
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-08-01 16:02 PDT by Philip Taylor
Modified: 2007-03-20 17:29 PDT (History)
6 users (show)
dveditz: blocking1.8.1.1+
dveditz: blocking1.8.0.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
image for test case (1.44 KB, image/png)
2006-08-01 16:03 PDT, Philip Taylor
no flags Details
test case (296 bytes, text/html)
2006-08-01 16:04 PDT, Philip Taylor
no flags Details
unexpected output (2.43 KB, image/png)
2006-08-01 16:07 PDT, Philip Taylor
no flags Details
image for new test case (320 bytes, image/png)
2006-09-17 09:59 PDT, Philip Taylor
no flags Details
new test case (709 bytes, text/html)
2006-09-17 10:01 PDT, Philip Taylor
no flags Details
grab the correct alpha depth from linux nsIImage (1.76 KB, patch)
2006-11-28 15:15 PST, Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail))
pavlov: review+
pavlov: superreview+
dveditz: approval1.8.0.9+
dveditz: approval1.8.1.1+
Details | Diff | Splinter Review

Description User image Philip Taylor 2006-08-01 16:02:10 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060801 BonEcho/2.0b1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060801 BonEcho/2.0b1

When drawing a 24-bit PNG (not indexed colour) with an alpha channel that can be represented in 1 bit (i.e. it has some 0, some 255, and nothing in between), the output is corrupted - it appears to squash the alpha channel sideways, and repeat it eight times across the width of the image. It looks very much like it's packing the alpha channel into bits, and then reading it back as bytes (plus reading random garbage towards the bottom of the picture when it reads past the alpha channel's end, which could perhaps be considered a (rather minor) security issue when combined with getImageData...)

This works correctly on Windows, but fails on Linux. I don't believe it is the same as bug 320510 - that affected indexed-colour PNGs/GIFs on Windows and seems fixed in Firefox 2.0, whereas this affects 24-bit PNGs on Linux in FF1.5 and 2.0. (And between the two different problems, it's a bit tricky making games with any kind of masked image that they can correctly draw in multiple browser versions, which makes sprites fun ;-) )

Reproducible: Always

Steps to Reproduce:
1. View the forthcoming test case.


Actual Results:  
An approximation of a slightly ugly circular smiley face, but with seven chunks taken out of the top and bottom (between the eight repetitions of the circular alpha channel).

Expected Results:  
A slightly ugly circular smiley face.
Comment 1 User image Philip Taylor 2006-08-01 16:03:33 PDT
Created attachment 231669 [details]
image for test case
Comment 2 User image Philip Taylor 2006-08-01 16:04:32 PDT
Created attachment 231670 [details]
test case
Comment 3 User image Philip Taylor 2006-08-01 16:07:08 PDT
Created attachment 231672 [details]
unexpected output
Comment 4 User image Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) 2006-09-08 08:34:49 PDT
I can confirm with a non-cairo build, but the problem seems to be gone in cairo ones.  Philip, can you confirm that with a trunk Firefox build?
Comment 5 User image Philip Taylor 2006-09-08 09:34:01 PDT
Confirmed - the test case works correctly for me in
  Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060908 Minefield/3.0a1
It still fails (as in the "unexpected output" case) with
  Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060908 BonEcho/2.0b2
and fails the same with
  Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.8.0.5) Gecko/20060731 Ubuntu/dapper-security Firefox/1.5.0.5

The situation where I encountered this problem (the sprites in http://canvex.lazyilluminati.com/) has the same behaviour, working on the trunk but not on the earlier branches.
Comment 6 User image Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) 2006-09-08 20:50:12 PDT
Ah, excellent.  Marking worksforme.
Comment 7 User image Philip Taylor 2006-09-17 09:59:58 PDT
Created attachment 238901 [details]
image for new test case
Comment 8 User image Philip Taylor 2006-09-17 10:01:31 PDT
Created attachment 238902 [details]
new test case
Comment 9 User image Philip Taylor 2006-09-17 10:12:13 PDT
I've added a new test case (and reopened the bug) because it seems to cause a not-entirely-trivial security problem in Firefox 2.0 (on Linux). The example has a 32767x1 image which triggers the incorrect behaviour - the alpha channel is (I assume) stored in 32K bits, but 32K bytes are drawn, with most of them being whatever happens to be in the heap after the image data. getImageData allows that data to be read back by the script (which works anywhere on the web, as long as it's the same domain as the image file).

When running the test case several times, I see various bits of dictionary and URLs and font filenames and HTML contents of other tabs, which is probably not a good thing to give scripts access to.
Comment 10 User image Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) 2006-09-17 11:35:04 PDT
Er, yes.  That would be very very bad.  Sounds like this is a branch-only security bug at this point....
Comment 11 User image Daniel Veditz [:dveditz] 2006-09-19 15:57:40 PDT
Restoring lost blocking flag
Comment 12 User image Daniel Veditz [:dveditz] 2006-11-03 11:11:06 PST
Vlad or Stuart: can one of you look at this or assign to a willing helper?
Comment 13 User image Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail)) 2006-11-07 14:40:58 PST
Ugh.  I'll look at this this week, it should be a straightforward fix.
Comment 14 User image Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail)) 2006-11-28 15:15:45 PST
Created attachment 246856 [details] [diff] [review]
grab the correct alpha depth from linux nsIImage

Stuart, does this look right?  Is there anything else that needs to be handled?

Also, do we want this code on for other platforms, or just GTK2?  As far as I can tell, nothing else lies about the image format in the same way.
Comment 15 User image Stuart Parmenter 2006-11-28 15:18:10 PST
Comment on attachment 246856 [details] [diff] [review]
grab the correct alpha depth from linux nsIImage

looks good to me.
Comment 16 User image Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail)) 2006-11-28 15:19:45 PST
Comment on attachment 246856 [details] [diff] [review]
grab the correct alpha depth from linux nsIImage

Requesting 1.8.1.1 approval; should be a low risk privacy fix.  Separate patch (but with essentially the same code) coming for 1.8.0.9.
Comment 17 User image Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail)) 2006-11-28 15:20:57 PST
Comment on attachment 246856 [details] [diff] [review]
grab the correct alpha depth from linux nsIImage

Actually, the patch is identical for 1.8.0.9 as well; requesting approval for there.
Comment 18 User image Daniel Veditz [:dveditz] 2006-11-29 10:51:12 PST
Comment on attachment 246856 [details] [diff] [review]
grab the correct alpha depth from linux nsIImage

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Comment 19 User image Vladimir Vukicevic [:vlad] [:vladv] (not actively reading bugmail)) 2006-11-29 13:23:40 PST
Checked in to 1.8.0.9 and 1.8.1.1.
Comment 20 User image alice nodelman [:alice] [:anode] 2006-11-30 16:47:33 PST
verified 'new test case' on fedora core release 5, checked that test case fails for 2.0.0.0 release.

verified 1.8.0.9
1.5.0.9pre 20061130

verified 1.8.1.1
2.0.0.1pre 2006113004

Note You need to log in before you can comment on or make changes to this bug.