Last Comment Bug 157989 - Possible heap corruption with 0-width GIF
: Possible heap corruption with 0-width GIF
Status: VERIFIED FIXED
[adt1 RTM] [ETA 07/24]
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: x86 Linux
: P1 major (vote)
: mozilla1.0.1
Assigned To: Stuart Parmenter
: Terri Preston
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: 143047 1.0.1
  Show dependency treegraph
 
Reported: 2002-07-17 14:30 PDT by Mitchell Stoltz (not reading bugmail)
Modified: 2002-09-05 16:29 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stop memory overwrite (936 bytes, patch)
2002-07-19 10:45 PDT, ArronM (:paper)
pavlov: review+
tor: superreview+
brendan: approval+
Details | Diff | Splinter Review
0 width header and frame (104 bytes, image/gif)
2002-07-19 10:50 PDT, ArronM (:paper)
no flags Details

Description Mitchell Stoltz (not reading bugmail) 2002-07-17 14:30:37 PDT
Submitter name:                zen-parse
   Submitter email address:       zen-parse@gmx.net
   Acknowledgement checkbox:      on
   Product:                       Netscape 6.x
   Operating system:              Unix: x86 Linux
   OS version:                    redhat 7.0
   Issue summary:                 a "0 width" GIF image: exploitable heap overwrite.

Issue details:
A specially formed gif image with width 0 can be 
used to overwrite 'chunk' information and other 
heap based information.

By overwriting a function pointer with an 
address containing user supplied data, it is 
possible to execute arbitary code.

http://lxr.mozilla.org/mozilla/source/modules/libimg/gifcom/gif.cpp#1274

down to line 1310
and then line 1351 and 1354


This form was submitted from
http://help.netscape.com/forms/bug-security.html?cp=bbpctr
with Opera/6.0 (Linux 2.2.19-7.0.16 i686; U)  [en].
Comment 1 Mitchell Stoltz (not reading bugmail) 2002-07-17 14:31:44 PDT
We need to know if this is really exploitable.
Comment 2 Stuart Parmenter 2002-07-17 16:41:04 PDT
Paper?  Can you take a look at this?
Comment 3 Jaime Rodriguez, Jr. 2002-07-18 09:55:46 PDT
Removing adt1.0.1 until there is a patch with reviews attached.
Comment 4 ArronM (:paper) 2002-07-19 10:45:07 PDT
Created attachment 91974 [details] [diff] [review]
stop memory overwrite

Firstly, gifcom is dead :)  It's been moved to /modules/libpr0n/decoders/gif/
which still contains a lot of the old gifcom code.

I talked it over with tor, and stopping the 0 width before the memory
allocation seemed to be the best solution.  And this is the patch.

The memory overwrite actually happens later on during do_lzw, which first
writes one byte, incrememnts the pointer, then checks whether it hit the end of
the buffer.  I looked into fixing the problem there, but it just caused
problems in other areas (namely in BeginGIF(), which will never initialize
mImageContainer if the width or height is 0.. which trickles down to assertion
errors)

I've also plugged the memory overwrite hole in do_lzw.	In theory, the width
check part of the patch should stop do_lzw from ever being called when the
buffer is 0 in length, but it's better to check anyway.
Comment 5 ArronM (:paper) 2002-07-19 10:50:20 PDT
Created attachment 91976 [details]
0 width header and frame

This doesn't do anything malicious, however, it does crash Mozilla.  I took a
14x14 GIF with one frame, and edited the image header and frame header so that
it says 0x0.
Comment 6 Jaime Rodriguez, Jr. 2002-07-19 11:41:08 PDT
seems like we have a patch from Arron M. 

dan/mitch: can you review and super-review the patch, so we can try and get it
landed asap. thanks!
Comment 7 Stuart Parmenter 2002-07-19 13:53:46 PDT
Comment on attachment 91974 [details] [diff] [review]
stop memory overwrite

r=pavlov
Comment 8 tor 2002-07-19 14:24:46 PDT
Comment on attachment 91974 [details] [diff] [review]
stop memory overwrite

sr=tor
Comment 9 Jaime Rodriguez, Jr. 2002-07-19 14:53:45 PDT
looks like we have the reviews.

pav: are you in a position to land this patch for aaron m?
Comment 10 Jaime Rodriguez, Jr. 2002-07-20 12:45:05 PDT
who's in a position to apply this patch to the trunk, so we can get some bake
time on this change?
Comment 11 Brendan Eich [:brendan] 2002-07-20 13:11:48 PDT
Comment on attachment 91974 [details] [diff] [review]
stop memory overwrite

Mixed indentation (2 vs. 4 space), but that seems to pervade the file already.

a=brendan@mozilla.org for 1.1beta trunk checkin.

/be
Comment 12 Paul Wyskoczka 2002-07-22 17:03:48 PDT
Adding adt1.0.1+ on behalf of the adt.  Please get Drivers approval for the
branch and check the fix into the branch.
Comment 13 chris hofmann 2002-07-22 20:28:14 PDT
a=chofmann for 1.0.1. add  the fixed1.0.1 keyword after getting this on the branch.
Comment 14 Stuart Parmenter 2002-07-22 20:30:59 PDT
landed on trunk and branch.
Comment 15 Jaime Rodriguez, Jr. 2002-07-22 20:35:54 PDT
thanks pav.

tpreston: teri, can you verify this fix on the branch, then replace "fixed1.0.1"
with "verified1.0.1"? thanks!
Comment 16 Terri Preston 2002-07-23 09:31:55 PDT
Verified fix checked into bonsai.mozilla.org and lxr.mozilla.org

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