963 bytes, application/octet-stream
1.49 KB, application/octet-stream
7.01 KB, patch
Christopher Aillon (sabbatical, not receiving bugmail): approval-aviary1.0.5+
Christopher Aillon (sabbatical, not receiving bugmail): approval1.7.9+
chris hofmann: approval1.8b2+
|Details | Diff | Splinter Review|
7.01 KB, patch
|Details | Diff | Splinter Review|
4.43 KB, patch
|Details | Diff | Splinter Review|
!!THIS INFORMATION IS EMBARGOED UNTIL 1PM BST 14/6/2005!! Please find attached a draft of the NISCC Vulnerability Advisory Notice (VAN) for NISCC Vulnerability #891011; please note that the information is shared under the terms of the Framework Agreement which you have agreed to previously. THIS IS NOT TO BE PASSED ON TO ANY THIRD PARTY WITHOUT PRIOR AGREEMENT FROM THE NISCC VULNERABILITY TEAM. NISCC will be disclosing this to the general public at 1PM BST 14/6/2005. You are all reminded that on the day of public release, the NISCC Vulnerability page will receive a large number of hits, therefore by passing NISCC a statement for inclusion on this page, you will be targeting a high proportion of your customers; hence if you wish to do so, statements will be accepted up to 4pm BST 13/6/2005 and then will be updated once daily. Please state whether any statement is for public release via the NISCC website or intended to be private information for NISCC only. Many thanks for your co-operation with NISCC on the handling of this vulnerability. ______________________________________________________________________Draft NISCC Vulnerability Advisory 891011/NISCC/IMAGEFORMATS Vulnerability Issues with the Parsing of Various Image Formats by Web Browsers ------------------- Advisory Reference 891011/NISCC/IMAGEFORMATS Release Date 14 June 2005 Last Revision 11 May 2005 Version Number 0.2 Acknowledgement --------------- These issues were identified using the Images Test Tool developed by Codenomicon Ltd.
Created attachment 184486 [details] windows .ico crashers (.zip) Test images that crash Firefox on windows XP SP2
Note: I didn't include the full text of the advisory which was non-browser-specific and mostly describes the tool they used (Sounds a bit like "mangleme" for image formats). It will be public soon enough. The salient fact for us are the test images. We may want to spin off sub-bugs.
I couldn't reproduce the crash with 00000038.ico in 1.0.4, the other two crash the 1.0 branch. On the trunk errors are correctly reported with all three images due to the fixes from bug 245631 -- go Son Le!
Oh, nice, that's the bug I've been trying to get into the branches for a bit. :-) I'm still for taking that patch on branches.
Reassigning to biesi, there's more of his fingerprints on XBM than Pav's.
OK. so all of these XBM files crash with SIGABRT here, because operator new threw an OOM exception (std::bad_alloc I believe). If I change the two new operators in nsXBMDecoder.cpp to new (std::nothrow) (and include <new>), then I don't crash.
I got a similar crash on a personal windows build using VC71, because unlike the release versions shipped using VC6 later versions of vc will also throw on new as required by the standard. It can be turned off by compiler option, but it's probably safer to go with least-common-denominator malloc for these simple byte arrays rather than worry about which compilers support which features.
Created attachment 184530 [details] [diff] [review] use malloc/free
Comment on attachment 184530 [details] [diff] [review] use malloc/free sr=dveditz
Can we get this in the alpha? It'd be good if it were there, but if so check in with a vague-ish comment like "use malloc to avoid OOM exceptions thrown by libstdc++". Don't mention the NISCC announcement.
Comment on attachment 184530 [details] [diff] [review] use malloc/free fixes crashes on large images on non-msvc6 builds. should be low risk (just uses malloc/calloc instead of new)
Comment on attachment 184530 [details] [diff] [review] use malloc/free a=chofmann
Fix checked into trunk for the Deer Park Alpha
Comment on attachment 184530 [details] [diff] [review] use malloc/free a=caillon for the branches
checked in to MOZILLA_1_7_BRANCH: Checking in bmp/nsBMPDecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp,v <-- nsBMPDecoder.cpp new revision: 188.8.131.52; previous revision: 184.108.40.206 done Checking in bmp/nsICODecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.cpp,v <-- nsICODecoder.cpp new revision: 220.127.116.11; previous revision: 1.30 done Checking in xbm/nsXBMDecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp,v <-- nsXBMDecoder.cpp new revision: 18.104.22.168; previous revision: 1.10 done
fixed on AVIARY_1_0_1_20050124_BRANCH: Checking in modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp,v <-- nsBMPDecoder.cpp new revision: 22.214.171.124.126.96.36.199; previous revision: 188.8.131.52.2.1 done Checking in modules/libpr0n/decoders/bmp/nsICODecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.cpp,v <-- nsICODecoder.cpp new revision: 184.108.40.206; previous revision: 1.30 done Checking in modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp,v <-- nsXBMDecoder.cpp new revision: 220.127.116.11; previous revision: 1.10 done
Created attachment 185265 [details] [diff] [review] branch patch this is the patch I checked in to the branches (the trunk patch didn't apply quite cleanly, because bug 245631 didn't land on the branches)
I still got crashes with both trunk and branch on Fedora Core 3 and Suse. Like this, The program 'Gecko' received an X Window System error. This probably reflects a bug in the program. The error was 'BadAlloc (insufficient resources for operation)'. (Details: serial 33675 error_code 11 request_code 53 minor_code 0) (Note to programmers: normally, X errors are reported asynchronously; that is, you will receive the error a while after causing it. To debug your program, run it with the --sync command line option to change this behavior. You can then get a meaningful backtrace from your debugger if you break on the gdk_x_error() function.) The firefox I tested, http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2005-06-06-12-trunk-fs/ http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2005-06-06-08-aviary1.0.1/ Did I miss something?
The crash is in nsImageGTK.cpp Line 176, mImageBits = (PRUint8*) new PRUint8[mSizeImage]; I changed it to malloc, it didn't crash with gdb, but it still crashed without gdb. I'm using debug build.
Created attachment 185653 [details] [diff] [review] patch for nsImageGTK.cpp use malloc/free in nsImageGTK.cpp
After my patch, mozilla won't crash on Solaris, but still crash on Linux. It crashed at line 415 of nsImageGTK.cpp (line 411 before patch) gdk_draw_rgb_image_dithalign(mImagePixmap, sXbitGC, rect->x, rect->y, rect->width, rect->height, GDK_RGB_DITHER_MAX, mImageBits + mRowBytes*rect->y + 3*rect->x, mRowBytes, rect->x, rect->y); I don't know how to deal with it. My linux machine has 512M RAM, the malloc for hundreds of millions bytes just returns a valid address.
Comment on attachment 185653 [details] [diff] [review] patch for nsImageGTK.cpp >the malloc for hundreds of millions bytes just >returns a valid address. yeah, linux sucks like that...
(In reply to comment #23) > yeah, linux sucks like that... > If I change xbm_width and xbm_height to 11383, I got crashes on Solaris too. Sometimes 'BadAlloc', sometimes 'BadDrawable (invalid Pixmap or Window parameter)' I think this problem is still there, if malloc returns not null. Memory is run out, the system is fragile. BTW: I wonder why this is a vulnerability.
(In reply to comment #24) > BTW: I wonder why this is a vulnerability. Because NISCC has classed it as such. They only told us about the problems (crashes) in our own program, perhaps another image-rendering program they tested is actually exploitable. Depending on how you look at it a potential DOS attack *is* a vulnerability.
should patch nsImageGTK.cpp to avoid OOM exceptions BTW: this is published at http://www.niscc.gov.uk/niscc/docs/al-20050614-00488.html
Let's get the latest patch reviewed and see what other work needs to be done so we can get this checked in soon, since this vulnerability went public recently.
Comment on attachment 185653 [details] [diff] [review] patch for nsImageGTK.cpp sr=dveditz a=dveditz for trunk and branches. What about the equivalent spots in all the other platform gfx nsImageFoo ?
(In reply to comment #28) > What about the equivalent spots in all the other platform gfx nsImageFoo ? That is, most of these spots: http://lxr.mozilla.org/mozilla/search?string=new%20PRUint8 (I notice we missed one in nsICODecoder.cpp, too)
What about 325 mColors = new colorTable[mNumColors]; 326 if (!mColors) 327 return NS_ERROR_OUT_OF_MEMORY; in nsICODecoder.cpp?
MOZILLA_1_7_BRANCH: Checking in gfx/src/gtk/nsImageGTK.cpp; /cvsroot/mozilla/gfx/src/gtk/nsImageGTK.cpp,v <-- nsImageGTK.cpp new revision: 18.104.22.168; previous revision: 22.214.171.124 done AVIARY_1_0_1_20050124_BRANCH: Checking in nsImageGTK.cpp; /cvsroot/mozilla/gfx/src/gtk/nsImageGTK.cpp,v <-- nsImageGTK.cpp new revision: 126.96.36.199.188.8.131.52; previous revision: 184.108.40.206.2.2 done trunk: Checking in nsImageGTK.cpp; /cvsroot/mozilla/gfx/src/gtk/nsImageGTK.cpp,v <-- nsImageGTK.cpp new revision: 1.149; previous revision: 1.148 done
Now public: http://www.niscc.gov.uk/niscc/docs/al-20050614-00488.html Opening bug. Note that this is just a DoS, and not exploitable.
This vulnerability is fixed, please log new bugs for any remaining issues.
v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9) Gecko/20050706 Firefox/1.0.5 using windows .ico images. No crash. Since we saw more problems on Linux, we need to also verify this on that platform.
*** Bug 238401 has been marked as a duplicate of this bug. ***