Last Comment Bug 164695 - Heap corruption in libjar using manifest length -1
: Heap corruption in libjar using manifest length -1
Status: VERIFIED FIXED
[sg:blocker]
:
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Daniel Veditz [:dveditz]
: bsharma
Mentors:
Depends on:
Blocks: 1.2
  Show dependency treegraph
 
Reported: 2002-08-26 14:56 PDT by Mitchell Stoltz (not reading bugmail)
Modified: 2002-12-20 16:45 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
jar-exploit-2.tar.gz (6.15 KB, application/x-gzip)
2002-09-02 10:39 PDT, Daniel Veditz [:dveditz]
no flags Details
prevent attack (plus fix unrelated nearby memory leak) (885 bytes, patch)
2002-10-28 13:38 PST, Daniel Veditz [:dveditz]
security-bugs: review+
darin.moz: superreview+
roc: approval+
Details | Diff | Review

Description Mitchell Stoltz (not reading bugmail) 2002-08-26 14:56:06 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:                 nsJAR::LoadEntry - heap corruption - code
execution

Issue details:
By supplying a malformed jar file, where the 
manifest file is described as being (unsigned 
int)-1 bytes long, it is possible to cause 
exploitable heap corruption.

(This is not the same as the other jar 
problem.)

[root@clarity secjars]# unzip -v jar.jar 
Archive:  jar.jar
 Length   Method    Size  Ratio   Date   Time 
  CRC-32    Name
--------  ------  ------- -----   ----   ---- 
  ------    ----
     268  Defl:X      199  26%  08-27-02 
04:21  18dc0dd8  page.html
      56  Stored 4294967295 -7669483%  
08-27-02 06:22  8cb14d9d  META-INF/manifest.mf
--------          -------  ---                
            -------
     324              198  39%                
            2 files




mozilla/modules/libjar/nsJAR.cpp

R::LoadEntry(const char* aFilename, char** 
aBuf, PRUint32* aBufLen)
...
  //-- Read the manifest file into memory
  char* buf;
  PRUint32 len;
  rv = manifestStream->Available(&len);
...
length now contains 4294967295 (== (int)-1)
...
  if (NS_FAILED(rv)) return rv;
  buf = (char*)PR_MALLOC(len+1);
...
allocated (-1+1)==0 bytes. 
(PR_MALLOC converts that into 1 byte)
...
  if (!buf) return NS_ERROR_OUT_OF_MEMORY;
  PRUint32 bytesRead;
  rv = manifestStream->Read(buf, len, 
&bytesRead);
...
and now it reads the rest of the file into 
that buffer.

There is now a heap based overflow

It is exploitable in a similar manner to the 
other jar problem I submitted. 

http://bugzilla.mozilla.org/show_bug.cgi?id=157646

(There is a similar problem in 
mozilla/security/nss/lib/jar/jarfile.c, but I 
am not sure how (if it is possible) to 
trigger it.)


This form was submitted from
http://help.netscape.com/forms/bug-security.html?cp=bbpctr
with Mozilla/4.0 (compatible; MSIE 5.0; Linux) Opera 6.03  [en].
Comment 1 Daniel Veditz [:dveditz] 2002-08-29 18:33:28 PDT
I can see how you've corrupted the heap, but I don't see how you turned that
into an exploit.
Comment 2 Daniel Veditz [:dveditz] 2002-09-02 10:39:05 PDT
Created attachment 97534 [details]
jar-exploit-2.tar.gz

Demo from zen-parse
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2002-10-15 09:00:29 PDT
This is easy to fix and it must be fixed for 1.2 final.

Dan, heap corruption can be turned into an exploit in some cases. It's a ton of
work but we can't discount it.
Comment 4 Christopher Blizzard (:blizzard) 2002-10-25 10:04:52 PDT
Anyone working on this?  This is a 1.2 blocker.
Comment 5 Daniel Veditz [:dveditz] 2002-10-28 13:38:46 PST
Created attachment 104404 [details] [diff] [review]
prevent attack (plus fix unrelated nearby memory leak)
Comment 6 Mitchell Stoltz (not reading bugmail) 2002-10-28 14:30:35 PST
Comment on attachment 104404 [details] [diff] [review]
prevent attack (plus fix unrelated nearby memory leak)

Dan, is -1 the only possible value that can be exploited? Is it ever legitimate
for this value to be negative? We might want to be extra safe and check for any
nonsensical value, not just -1.

If that's the only value that could possibly cause  heap corruption, then
r=mstoltz
Comment 7 Daniel Veditz [:dveditz] 2002-10-28 15:52:40 PST
The size value is not actually -1, it's unsigned. When you add 1 to it (to
account for the null) it overflows; any other large value is OK.

Would be nice to re-write this to keep track of the file size rather than rely
on null termination.
Comment 8 Darin Fisher 2002-10-29 10:33:17 PST
Comment on attachment 104404 [details] [diff] [review]
prevent attack (plus fix unrelated nearby memory leak)

>Index: nsJAR.cpp

>+  if (len == 0xFFFFFFFF)
>+    return NS_ERROR_FILE_CORRUPTED; // bug 164695

nit: how about PRUint32(-1) instead?  that way no one has to count digits :-/

either way, sr=darin.
Comment 9 Daniel Veditz [:dveditz] 2002-10-29 18:38:52 PST
This should go on the branch.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2002-10-29 20:24:56 PST
Comment on attachment 104404 [details] [diff] [review]
prevent attack (plus fix unrelated nearby memory leak)

a=roc+moz for trunk
Comment 11 Michael Buckland 2002-10-30 17:51:32 PST
Discussed in bBird team meeting.  Plussing for adt only.
Comment 12 Daniel Veditz [:dveditz] 2002-10-30 22:34:03 PST
Checked into trunk
Comment 13 chris hofmann 2002-10-31 08:52:12 PST
a=chofmann 1.0.2
Comment 14 Daniel Veditz [:dveditz] 2002-11-01 22:45:58 PST
Checked into branch
Comment 15 bsharma 2002-11-20 13:06:35 PST
Verified on 2002-11-20-branch build on Linux. Loaded the attached test case and
the crash does not happen.The page shows up with the line streaks.

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