Bug 532380 - Buffer overflow in zipinfo
Summary: Buffer overflow in zipinfo
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: unzip
Version: 11
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Karel Klíč
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-01 23:38 UTC by Doncho Gunchev
Modified: 2014-11-03 19:20 UTC (History)
6 users (show)

Fixed In Version: 5.52-11.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-12-25 08:22:54 UTC


Attachments (Terms of Use)
Test file with bad hostver (146 bytes, application/zip)
2009-11-22 19:37 UTC, Tomas Hoger
no flags Details

Description Doncho Gunchev 2009-11-01 23:38:13 UTC
Description of problem:
Buffer overflow on 'ACORN_' (os?).

Version-Release number of selected component (if applicable):
unzip-5.52-10.fc11.x86_64, unzip-5.52-3.el5

How reproducible:
Always.

Steps to Reproduce:
1. Get a zip file with version/os > 9 (see below)
2. run zipinfo on that file
  
Actual results:
*** buffer overflow detected ***: zipinfo terminated

Expected results:
normal execution no matter what archive or file you give (error message, not buffer overflow).

Additional info:
Happens on RHEL, CentOS and Fedora. I can try to provide you with the zip file, but I think it's obvious. I have no idea of the possible impact so I mark this as security sensitive, feel free to edit.

/usr/src/debug/unzip-5.52/zipinfo.c:
1957│         case ACORN_:
1958│             if (hostnum != FS_FAT_ ||
1959│                 (unsigned)(xattr & 0700) !=
1960│                  ((unsigned)0400 |
1961│                   ((unsigned)!(G.crec.external_file_attributes & 1) << 7) |
1962│                   ((unsigned)(G.crec.external_file_attributes & 0x10) << 2))
1963│                )
1964│             {
1965├───────────────> xattr = (unsigned)(G.crec.external_file_attributes & 0xFF);
1966│                 sprintf(attribs, ".r.-...     %u.%u", hostver/10, hostver%10);

(gdb) p sizeof(attribs)
$6 = 16
(gdb) p hostver
$7 = 100
(gdb) p attribs
$19 = ".r.-...     10."

Comment 1 Tomas Hoger 2009-11-02 07:55:30 UTC
If you already have a zip file triggering this flaw, please attach.  It can help testing.

Btw, your arrow pointing at the problem seems to be one line early.  There are also few more occurrences of sprintf(attribs, ... , hostver/10, ...) pattern in the same file.

Have you tried reporting this upstream already?

Comment 2 Tomas Hoger 2009-11-02 08:38:32 UTC
(In reply to comment #0)
> I have no idea of the possible impact so I mark this as security
> sensitive, feel free to edit.

As it's sprintf to static char buffer, but should be caught by FORTIFY_SOURCE reducing impact to the harmless crash.  Your "Actual results" confirms that too.  It may be an issue for older RHELs, but the overflow may be limited to only overwrite other local variables.  From a quick look into the source:

   1751     unsigned    hostnum, hostver, methnum, xattr;
   1752     char        *p, workspace[12], attribs[16];

So the memory right behind attribs should be used by hostnum / hostver.  As hostver is unsigned int (32bit on both 32 and 64 bit arches, with max value of 4294967295), so hostver/10 should be 9 chars at max, out of which 1 has already space reserved in attribs.  This should limit overflow to 8 bytes overwriting hostnum and hostver, which are read from the file anyway.

Of course, compiler optimizations may break this theory.

Comment 3 Vincent Danen 2009-11-12 17:03:49 UTC
(In reply to comment #0)
> Happens on RHEL, CentOS and Fedora. I can try to provide you with the zip file,

Doncho, would it be possible to have the zip file you found this with to help in reproducing the issue and, more importantly, verifying a fix?  It would be incredibly helpful if you could provide the zip file you observed this behaviour with.

Thanks.

Comment 4 Ivana Varekova 2009-11-18 12:39:36 UTC
Hello, the package maintainer was changed to Karel Klic so I'm reassignig bug to him.

Comment 5 Tomas Hoger 2009-11-22 19:35:19 UTC
It turns out I wasn't completely correct in comment #2.  Relevant coda part from the zi_short (zipinfo.c) with attribs declaration for reference:

  1790     unsigned    hostnum, hostver, methnum, xattr;
  1791     char        *p, workspace[12], attribs[16];
  1792     char        methbuf[5];

I mentioned that the overflow is limited to 8 characters (7 numeric + '\0') based on the hostver unsigned int type.  However, host OS version number recorded in zip file is 1 byte.  So the maximum value is 255, which results in a single '\0' overflow.

On builds without FORTIFY_SOURCE, that byte overwrites the first byte of either workspace (x86 arches) or methbuf (non-x86 rhel arches), based on how compiler lays out those buffers.

workspace is only used for VMS hostnum and is no longer used after the point it is overwritten.  methbuf is printed as part of the second line of the zipinfo output, so '\0' as the first byte causes "compression method" not to be printed, which can be seen if you compare zipinfo output on e.g. i386 and ia64:

Archive:  foo.zip   146 bytes   1 file
-rw-------  25.5 unx        0 bx stor 21-Nov-09 14:06 foo.txt
1 file, 0 bytes uncompressed, 0 bytes compressed:  0.0%

Archive:  foo.zip   146 bytes   1 file
-rw-------  25.5 unx        0 bx  21-Nov-09 09:06 foo.txt
1 file, 0 bytes uncompressed, 0 bytes compressed:  0.0%

(note the missing "stor" in the second output)

So this is not exploitable even on older RHELs.  Karel, feel free to report upstream.  Increasing the size of attribs by 1 should be sufficient fix here.

Comment 6 Tomas Hoger 2009-11-22 19:37:30 UTC
Created attachment 372950 [details]
Test file with bad hostver

Comment 7 Fedora Update System 2009-11-30 11:30:03 UTC
unzip-5.52-12.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/unzip-5.52-12.fc12

Comment 8 Fedora Update System 2009-11-30 11:31:09 UTC
unzip-5.52-11.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/unzip-5.52-11.fc11

Comment 9 Karel Klíč 2009-11-30 11:36:39 UTC
Thank you for nicely detailed bug report.

I reported it to the upstream:
http://www.info-zip.org/board/board.pl?m-1259575993/

Comment 10 Fedora Update System 2009-12-01 04:23:03 UTC
unzip-5.52-11.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update unzip'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-12361

Comment 11 Doncho Gunchev 2009-12-06 16:49:17 UTC
unzip-5.52-11.fc12.x86_64 works, 10x.

Comment 12 Fedora Update System 2009-12-25 08:22:46 UTC
unzip-5.52-11.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2010-01-19 00:59:49 UTC
unzip-5.52-12.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


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