Last Comment Bug 695421 - Loading the web page http://backup42.zimfer.com/ crashes Gecko > 8
: Loading the web page http://backup42.zimfer.com/ crashes Gecko > 8
Status: VERIFIED FIXED
[qa!]
: crash, crashreportid, regression, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
http://backup42.zimfer.com/
Depends on: CVE-2012-3966
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-18 11:42 PDT by therube
Modified: 2012-07-21 14:56 PDT (History)
12 users (show)
netzen: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
-
fixed
-
fixed


Attachments
Fix for invalid ICO width causing crash (4.75 KB, patch)
2011-10-22 17:42 PDT, Brian R. Bondy [:bbondy]
joe: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Review
reftests for ICOs with invalid wdith & height specified (4.71 KB, patch)
2011-10-22 17:43 PDT, Brian R. Bondy [:bbondy]
joe: review+
Details | Diff | Review

Description therube 2011-10-18 11:42:55 PDT
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111018 Firefox/10.0a1
Build ID: 20111018031016

Steps to reproduce:

 
*CRASH WARNING*

Load   http://backup42.zimfer.com/

*CRASH WARNING*
 


Actual results:

 
Crash.

https://crash-stats.mozilla.com/report/index/bp-e921dc53-41d6-4a2f-b28d-1fb572111018
 


Expected results:

 
Should not crash.

This affects FF 9 & 10, SeaMonkey 2.6 & 2.7.
FF 8 & SeaMonkey 2.5 are not affected.
Comment 1 Robert Kaiser (not working on stability any more) 2011-10-18 11:48:12 PDT
Top frames:
0 	xul.dll 	mozilla::imagelib::nsICODecoder::WriteInternal 	image/decoders/nsICODecoder.cpp:530
1 	xul.dll 	mozilla::imagelib::RasterImage::WriteToDecoder 	image/src/RasterImage.cpp:2284
2 	xul.dll 	mozilla::imagelib::RasterImage::DecodeSomeData 	image/src/RasterImage.cpp:2618
3 	xul.dll 	mozilla::imagelib::imgDecodeWorker::Run 	image/src/RasterImage.cpp:2735
4 	xul.dll 	mozilla::imagelib::RasterImage::AddSourceData 	image/src/RasterImage.cpp:1281
5 	xul.dll 	mozilla::imagelib::RasterImage::WriteToRasterImage 	image/src/RasterImage.cpp:2833
6 	xul.dll 	nsInputStreamTee::WriteSegmentFun 	xpcom/io/nsInputStreamTee.cpp:223
7 	xul.dll 	nsPipeInputStream::ReadSegments 	xpcom/io/nsPipe3.cpp:799
8 	xul.dll 	nsInputStreamTee::ReadSegments 	xpcom/io/nsInputStreamTee.cpp:276
9 	xul.dll 	imgRequest::OnDataAvailable 	image/src/imgRequest.cpp:1149
10 	xul.dll 	ProxyListener::OnDataAvailable 	image/src/imgLoader.cpp:2095


Apparently, bug 682568 fixed something in that code but there's still a crash left here.
Comment 2 Robert Kaiser (not working on stability any more) 2011-10-18 11:49:43 PDT
CCing bbondy, who has worked on bug 682568.
Comment 3 Marcia Knous [:marcia - use ni] 2011-10-18 16:05:25 PDT
Confirming crash on Windows. I also crash on Mac ->  https://crash-stats.mozilla.com/report/index/bp-1996c682-c5c8-4f3e-90fe-a4c8f2111018
Comment 4 Alice0775 White 2011-10-18 20:04:38 PDT
Regression window(cached m-c),
Works:
http://hg.mozilla.org/mozilla-central/rev/cc1e08803869
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110826 Firefox/9.0a1 ID:20110826004925
Fails:
http://hg.mozilla.org/mozilla-central/rev/e8af0a8c3632
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110826 Firefox/9.0a1 ID:20110826092401
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc1e08803869&tochange=e8af0a8c3632


Regression window(cached m-i),
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9f11fc87eaa6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110825 Firefox/9.0a1 ID:20110825130335
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f097100df8aa
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110825 Firefox/9.0a1 ID:20110825131317
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9f11fc87eaa6&tochange=f097100df8aa

Triggered by:
1f86f6af9434	Brian R. Bondy — Bug 600556 - Support Vista-style PNG ICO files. r=joe
Comment 5 Daniel Holbert [:dholbert] 2011-10-19 16:33:37 PDT
No crash on 64-bit Linux for me, btw.
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111019 Firefox/10.0a1
Comment 6 Daniel Holbert [:dholbert] 2011-10-19 16:40:09 PDT
(In reply to Marcia Knous [:marcia] from comment #3)
> Confirming crash on Windows. I also crash on Mac -> 
The mac crash-stack ( bp-1996c682-c5c8-4f3e-90fe-a4c8f2111018 ) is completely different -- I wonder why...
Comment 7 Brian R. Bondy [:bbondy] 2011-10-22 12:41:50 PDT
The problem was with the ICO: 
http://backup42.zimfer.com/backup42.ico

The problem was with an invalid ICO width & height specified. 
Note: We didn't previously render the ICO from that that site correctly in any FF version, but that was better handling than crashing/memory corruption.

The fix is coming soon which will both render it correctly and fix the crash.
The fix is to use the bitmap information header width and height always over what we have in the ICO format.  The spec says that you should use the contained resource info over the ICO info.

> The mac crash-stack ( bp-1996c682-c5c8-4f3e-90fe-a4c8f2111018 ) is 
> completely different -- I wonder why...

There was memory corruption so that is why.
Comment 8 Brian R. Bondy [:bbondy] 2011-10-22 17:42:29 PDT
Created attachment 568917 [details] [diff] [review]
Fix for invalid ICO width causing crash

The width in the ICO image was twice what it was supposed to be, this caused problems in the 'AND mask' of the ICO.  Instead we should use the width of the contained BMP resource as the spec says.  FF <= 8 didn't crash but didnt' display correctly.  We now don't crash and also display correctly.
Comment 9 Brian R. Bondy [:bbondy] 2011-10-22 17:43:25 PDT
Created attachment 568918 [details] [diff] [review]
reftests for ICOs with invalid wdith & height specified
Comment 10 Joe Drew (not getting mail) 2011-11-03 15:11:50 PDT
Comment on attachment 568917 [details] [diff] [review]
Fix for invalid ICO width causing crash

Review of attachment 568917 [details] [diff] [review]:
-----------------------------------------------------------------

ICO makes me sad :(
Comment 11 Brian R. Bondy [:bbondy] 2011-11-03 18:52:39 PDT
> ICO makes me sad :(

Tell me about it :)
Comment 12 Brian R. Bondy [:bbondy] 2011-11-03 19:05:28 PDT
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=eff544f5e248
Comment 15 Brian R. Bondy [:bbondy] 2011-11-04 11:39:19 PDT
Comment on attachment 568917 [details] [diff] [review]
Fix for invalid ICO width causing crash

This is a fix for a crash that is a regression introduced with rewriting the ICO file format code.  It happens when there is an invalid ICO file with a large wrong width in the ICO header, but the correct width is in the BMP header.
I think it is important because anyone could create a webpage that crashes your browser with such an ICO file.

Risk of causing further problems: medium-low

I realize it is close to migration time so it may get processed after the migration. I would like to have this fix in v9 which is the first place it was introuced.  It is already fixed in v10 Nightly today.
Comment 16 christian 2011-11-07 13:44:22 PST
Comment on attachment 568917 [details] [diff] [review]
Fix for invalid ICO width causing crash

[triage comment]

This looks like a good fix and would be a regression shipped in Fx9. Approved for aurora. Please land today if at all possible.
Comment 17 Brian R. Bondy [:bbondy] 2011-11-07 13:45:38 PST
> Approved for aurora. Please land today if at all possible.

Is this message for someone else? I've never landed on anything other than m-i and m-c.
Comment 18 Brian R. Bondy [:bbondy] 2011-11-07 13:57:08 PST
Sorry last message was more for policy wise than for technically how to do it.
I'm doing this push to aurora now, will update shortly.
Comment 19 Brian R. Bondy [:bbondy] 2011-11-07 14:32:00 PST
Location of image lib has changed on that branch and also PRBool -> bool.  I'm going to rebuild and test it before pushing to Aurora.  So I'm not sure if the cut over will happen before I'm done.  But I'm doing this now.
Comment 20 Brian R. Bondy [:bbondy] 2011-11-07 16:31:31 PST
Pushed to Aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/9b37b23dbc4a
Comment 21 Paul Silaghi, QA [:pauly] 2011-11-22 04:46:40 PST
The issue is not reproducible on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111121 Firefox/11.0a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2

Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111121 Firefox/11.0a1
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111121 Firefox/10.0a2

Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 5.1; rv:11.0a1) Gecko/20111121 Firefox/11.0a1
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111121 Firefox/10.0a2

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111122 Firefox/11.0a1

Setting the resolution to verified fixed.

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