oss-sec mailing list archives
Re: CVE Request: Info-ZIP unzip 6.0
From: mancha <mancha1 () zoho com>
Date: Wed, 11 Feb 2015 12:27:06 +0000
On Tue, Feb 10, 2015 at 02:11:59PM +0100, Tomas Hoger wrote:
On Mon, 22 Dec 2014 18:14:58 +0000 mancha wrote:OOB access (both read and write) issues exist in test_compr_eb (extract.c) that can result in application crash or other unspecified impact. This vulnerability can be triggered via crafted zip archives with extra fields that advertise STORED method compression (i.e. no compression) and have uncompressed field sizes smaller than the corresponding compressed field sizes. This issue is different from CVE-2014-8140 [1].FWIW, those issues are not entirely different, as the oCERT-2014-011 reproducer triggered the same issue your patch addresses - memcpy() buffer overflow when using STORED compression and the uncompressed size field value smaller than the rest of the data in the extra field block. In case of the oCERT-2014-011 report, the size was special - 0. Your check, however, would prevent overflow on the test case. The check to reject uncompressed size of 0 is still needed to avoid bypassing check if extra field block still has enough data for the compression header. That makes it possible to bypass your check and trigger integer underflow in memextract() when EB_CMPRHEADLEN (2 + 4) is subtracted from srcsize, which leads to memcpy() with size close to SIZE_MAX. Your patch, unzip-6.0_overflow2.diff, which is what got applied upstream, seems to perform an incorrect check. It ensures that eb_ucsize is equal to eb_size - compr_offset. The latter value includes compression header length (EB_CMPRHEADLEN), which is not included in eb_ucsize AFAICT (based on what I could find in extrafld.txt or os2/os2zip.c in Zip 3.0 sources). It seems the check should be: (eb_size - compr_offset - EB_CMPRHEADLEN != eb_ucsize) Can you or upstream confirm? This problem would not be a security problem, but a bug that could cause well-formed extra fields to be rejected as invalid.
You're right. The identity that should hold in stored method is: eb_size - compr_offset - EB_CMPRHEADLEN == eb_ucsize What must have thrown me for a loop when churning out the patch is the weird wording in unzpriv.h: #define EB_OS2_HLEN 4 /* size of OS2/ACL compressed data header */ Thanks for catching that. I've removed the buggy patch from sf and replaced it with: http://sf.net/projects/mancha/files/sec/unzip-6.0_overflow3.diff --mancha
Attachment:
_bin
Description:
Current thread:
- Re: CVE Request: Info-ZIP unzip 6.0 mancha (Jan 20)
- Re: CVE Request: Info-ZIP unzip 6.0 cve-assign (Jan 22)
- <Possible follow-ups>
- Re: CVE Request: Info-ZIP unzip 6.0 Tomas Hoger (Feb 10)
- Re: CVE Request: Info-ZIP unzip 6.0 mancha (Feb 11)
- Re: CVE Request: Info-ZIP unzip 6.0 Steven M. Schweda (Feb 10)
- Re: CVE Request: Info-ZIP unzip 6.0 Steven M. Schweda (Feb 11)