Closed
Bug 164695
Opened 22 years ago
Closed 21 years ago
Heap corruption in libjar using manifest length -1
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
VERIFIED
FIXED
People
(Reporter: security-bugs, Assigned: dveditz)
References
Details
(Whiteboard: [sg:blocker])
Attachments
(2 files)
6.15 KB,
application/x-gzip
|
Details | |
885 bytes,
patch
|
security-bugs
:
review+
darin.moz
:
superreview+
roc
:
approval+
|
Details | Diff | Splinter Review |
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].
Assignee | ||
Comment 1•22 years ago
|
||
I can see how you've corrupted the heap, but I don't see how you turned that into an exploit.
Assignee | ||
Comment 2•22 years ago
|
||
Demo from zen-parse
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.
Whiteboard: [sg:blocker]
Blocks: 1.2
Comment 4•21 years ago
|
||
Anyone working on this? This is a 1.2 blocker.
Assignee | ||
Comment 5•21 years ago
|
||
Reporter | ||
Comment 6•21 years ago
|
||
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
Attachment #104404 -
Flags: review+
Assignee | ||
Comment 7•21 years ago
|
||
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•21 years ago
|
||
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.
Attachment #104404 -
Flags: superreview+
Comment on attachment 104404 [details] [diff] [review] prevent attack (plus fix unrelated nearby memory leak) a=roc+moz for trunk
Attachment #104404 -
Flags: approval+
Comment 11•21 years ago
|
||
Discussed in bBird team meeting. Plussing for adt only.
Assignee | ||
Comment 12•21 years ago
|
||
Checked into trunk
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 15•21 years ago
|
||
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.2 → verified1.0.2
Reporter | ||
Updated•21 years ago
|
Group: security
You need to log in
before you can comment on or make changes to this bug.
Description
•