Closed Bug 295457 Opened 19 years ago Closed 19 years ago

NISCC vulnerability #891011 (Parsing of Various Image Formats by Web Browsers)

Categories

(Core :: Graphics: ImageLib, defect, P1)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: dveditz, Assigned: Biesinger)

References

Details

(Keywords: fixed-aviary1.0.5, fixed1.7.9, Whiteboard: [sg:dos] public on June 14. ready for branch landing)

Attachments

(5 files)

!!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.
Test images that crash Firefox on windows XP SP2
Attachment #184486 - Attachment description: windows .ico crashers → windows .ico crashers (.zip)
Attachment #184487 - Attachment description: test images for SUSE linux 2.6.4-52 → test images for SUSE linux 2.6.4-52 (.zip)
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.
Whiteboard: [sg:fix] public on June 14
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!
Depends on: 245631
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.
Assignee: pavlov → cbiesinger
OS: Windows XP → All
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
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.
OS: All → Windows XP
Priority: P1 → --
Target Milestone: mozilla1.8beta2 → ---
Attached patch use malloc/freeSplinter Review
Attachment #184530 - Flags: superreview?(dveditz)
Attachment #184530 - Flags: review?(tor)
OS: Windows XP → All
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 184530 [details] [diff] [review]
use malloc/free

sr=dveditz
Attachment #184530 - Flags: superreview?(dveditz) → superreview+
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.
Flags: blocking1.8b2+
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.5+
Flags: blocking1.7.9+
Attachment #184530 - Flags: review?(tor) → review+
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[])
Attachment #184530 - Flags: approval1.8b2?
Comment on attachment 184530 [details] [diff] [review]
use malloc/free

a=chofmann
Attachment #184530 - Flags: approval1.8b2? → approval1.8b2+
Attachment #184530 - Flags: approval1.7.9?
Attachment #184530 - Flags: approval-aviary1.0.5?
Fix checked into trunk for the Deer Park Alpha
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [sg:fix] public on June 14 → [sg:fix] public on June 14. Need branch landing
Comment on attachment 184530 [details] [diff] [review]
use malloc/free

a=caillon for the branches
Attachment #184530 - Flags: approval1.7.9?
Attachment #184530 - Flags: approval1.7.9+
Attachment #184530 - Flags: approval-aviary1.0.5?
Attachment #184530 - Flags: approval-aviary1.0.5+
Whiteboard: [sg:fix] public on June 14. Need branch landing → [sg:fix] public on June 14. ready for branch landing
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: 1.24.2.3; previous revision: 1.24.2.2
done
Checking in bmp/nsICODecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.cpp,v  <-- 
nsICODecoder.cpp
new revision: 1.30.2.1; previous revision: 1.30
done
Checking in xbm/nsXBMDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/xbm/nsXBMDecoder.cpp,v  <-- 
nsXBMDecoder.cpp
new revision: 1.10.4.1; 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: 1.24.2.1.2.1.2.1; previous revision: 1.24.2.1.2.1
done
Checking in modules/libpr0n/decoders/bmp/nsICODecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.cpp,v  <-- 
nsICODecoder.cpp
new revision: 1.30.16.1; 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: 1.10.18.1; previous revision: 1.10
done
Attached patch branch patchSplinter Review
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.
use malloc/free in nsImageGTK.cpp
Attachment #185653 - Flags: superreview?(dveditz)
Attachment #185653 - Flags: review?(cbiesinger)
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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...
Attachment #185653 - Flags: review?(cbiesinger) → review+
(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 ?
Attachment #185653 - Flags: superreview?(dveditz)
Attachment #185653 - Flags: superreview+
Attachment #185653 - Flags: approval1.8b3+
Attachment #185653 - Flags: approval1.7.9+
Attachment #185653 - Flags: approval-aviary1.0.5+
(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)
Assignee: cbiesinger → pavlov
Status: REOPENED → NEW
Assignee: pavlov → cbiesinger
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: 1.131.2.3; previous revision: 1.131.2.2
done

AVIARY_1_0_1_20050124_BRANCH:
Checking in nsImageGTK.cpp;
/cvsroot/mozilla/gfx/src/gtk/nsImageGTK.cpp,v  <--  nsImageGTK.cpp
new revision: 1.131.2.1.2.2.2.1; previous revision: 1.131.2.1.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.
Group: security
Whiteboard: [sg:fix] public on June 14. ready for branch landing → [sg:dos] public on June 14. ready for branch landing
This vulnerability is fixed, please log new bugs for any remaining issues.
Flags: blocking-aviary1.0.6-
Status: NEW → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
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. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: