At Thu, 10 Jun 2004 10:11:17 +0900, GOTO Masanori wrote: > At Tue, 8 Jun 2004 10:32:48 -0700, > Matt Zimmerman wrote: > > On Sun, Jun 06, 2004 at 04:28:56PM -0000, lw@xxxxxxxxxxxx wrote: > > > In-Reply-To: <20040605203922.GW19402@xxxxxxxxx> > > > > > > i didn't bother to check deb package, but this patch: > > > > http://security.debian.org/pool/updates/non-free/l/lha/lha_1.14i-2woody1.diff.gz > > > applied to this package: > > > > Size/MD5 checksum: 21414 0f990fd920ea4770dd088a97c1c87f18 > > > > http://security.debian.org/pool/updates/non-free/l/lha/lha_1.14i.orig.tar.gz > > > > > > has not fixed this problem: > > > http://www.securityfocus.com/archive/1/363418 > > > POC is at: > > > http://lw.ftw.zamosc.pl/lha-exploit.txt > > Thanks for your checking, you're right and actually sample program > (lha-exploit.txt) causes the problem. > > > > http://bugs.gentoo.org/show_bug.cgi?id=51285 > > > is some dull discussion about that bug. > > This patch seems being appropriate. I'll check again, and I'll > prepare the next version for this problem. When I replied the above mail, I did not change three address in lha-exploit.txt . I changed the perl program described at http://lw.ftw.zamosc.pl/lha-exploit.txt for my environment. Then I found that the proposed patch at http://bugs.gentoo.org/show_bug.cgi?id=51285 fixes partially for this problem. When I used lha with "l" or "v" option: gotom@celesta> ./lha l ../../exploit-test/archive.lhz PERMSSN UID GID SIZE RATIO STAMP NAME ---------- ----------- ------- ------ ------------ -------------------- / ---------- ----------- ------- ------ ------------ -------------------- Total v: command not found 0 ****** Jun 10 12:40 drwxr-xr-x 0/0 0 ****** May 19 04:47 F/ drwxr-xr-x 0/0 0 ****** May 19 04:47 F/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/ drwxr-xr-x 0/0 0 ****** May 19 04:47 F/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/ drwxr-xr-x 0/0 0 ****** May 19 04:47 F/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/ [unknown] 0 ****** Sep 10 14:53 F/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/ [unknown] 0 ****** Sep 10 14:53 F/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/AAAA@@/ ---------- ----------- ------- ------ ------------ -------------------- Total v: command not found 0 ****** Jun 10 12:40 "command not found" is the result of execution system(). So if malicious local user creates /tmp/something (which is written in the exploit binary), and then root user or root process executes archive.tgz, the local user can do something possibly. This is because the "LzHeader hdr" in get_header(fp, hdr) is allocated on the stack. But directory is allowed by 1024 (for the first fix of this bug) in get_header() with type==2. But hdr->name has 256 bytes. So strcat() and strcpy() in get_header can make this problem. The patch from Ulf for fixing the original probelm, at header.c: case 2: /* * directory */ if (header_size >= FILENAME_LENGTH) { fprintf(stderr, "Possible buffer overflow hack attack, type #2\n"); exit(110); } lha_macro.h: #define FILENAME_LENGTH 1024 At the end of get_header in header.c: if (dir_length) { strcat(dirname, hdr->name); // potential buffer overflow! strcpy(hdr->name, dirname); // potential buffer overflow! name_length += dir_length; } The size of the maximum path name should be taken from pathconf(_PC_PATH_MAX), but SUSv3 says _POSIX_PATH_MAX=256 and _XOPEN_PATH_MAX=1024. In this case hdr->name has 256, so at least it's allowed for this problem. Looking at the LHA header, LHarc format had path length limitation up to 233 byte, but the newer format level 2 header has no limitation theoretically. "Possbile buffer overflow hack" is just _possible_, so the long pathname can be included in an lzh archive. However, generally speaking, the path length limitation up to 1024 or even 256 byte is allowable, I think. Of course, it's sure that the correct fix is to use variable length buffer for hdr->name using file_length and dir_length. So, from the above analysis, the proposed patch is safe for "x", but not for "l". The following patch should fix this problem for potential malicious archive with lha option "l" "v" "x". --- src/header.c 2002-07-19 17:23:58.000000000 +0900 +++ src/header.c 2004-06-16 09:49:23.000000000 +0900 @@ -648,8 +648,17 @@ } if (dir_length) { + if ((dir_length + name_length) > sizeof(dirname)) { + fprintf(stderr, "Insufficient buffer size\n"); + exit(112); + } strcat(dirname, hdr->name); - strcpy(hdr->name, dirname); + + if ((dir_length + name_length) > sizeof(hdr->name)) { + fprintf(stderr, "Insufficient buffer size\n"); + exit(112); + } + strncpy(hdr->name, dirname, sizeof(hdr->name)); name_length += dir_length; } Comments are welcome. Regards, -- gotom
