search

kyz / libmspack

A library for some loosely related Microsoft compression formats, CAB, CHM, HLP, LIT, KWAJ and SZDD.
https://www.cabextract.org.uk/libmspack/
169 stars 45 forks source link

cabextract NULL pointer dereference #16

Closed recvfrom closed 6 years ago

recvfrom commented 6 years ago

When I try to run cabextract against a certain CAB file I get the following output (from cabextract 1.6 installed via the Ubuntu 17.10 apt-get repo):

cabextract /tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8 
/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8: CAB: Folder record 0
/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8: CAB: Folder offset: 71
/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8: CAB: Folder compression method: 1
/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8: CAB: Recorded folders: 1
CAB: File record 0
CAB: File name: XXXXXX*exe
CAB: File offset: 0
CAB: File folder index: 4294967295
CAB: File attribs: 0x20
CAB:   * file modified since last backup
CAB: Recorded files: 1

/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8: can't find (null)
Extracting cabinet: /tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8
Segmentation fault (core dumped)

I built from source and confirmed that the issue still occurs. Here is the gdb output related to the crash:

(gdb) run
Starting program: /home/zelda/workspace/libmspack/cabextract/cabextract /tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8

Program received signal SIGSEGV, Segmentation fault.
__gconv (cd=0x0, inbuf=inbuf@entry=0x0, inbufend=inbufend@entry=0x0, outbuf=outbuf@entry=0x0, 
    outbufend=outbufend@entry=0x0, irreversible=irreversible@entry=0x7fffffffda30) at gconv.c:42
42  gconv.c: No such file or directory.
(gdb) bt
#0  __gconv (cd=0x0, inbuf=inbuf@entry=0x0, inbufend=inbufend@entry=0x0, outbuf=outbuf@entry=0x0, 
    outbufend=outbufend@entry=0x0, irreversible=irreversible@entry=0x7fffffffda30) at gconv.c:42
#1  0x00007ffff7a16b61 in iconv (cd=<optimized out>, inbuf=0x0, inbytesleft=0x0, outbuf=0x0, 
    outbytesleft=0x0) at iconv.c:42
#2  0x0000555555555ddf in convert_filename (name=<optimized out>) at src/cabextract.c:1004
#3  convert_filenames (files=<optimized out>) at src/cabextract.c:1025
#4  process_cabinet (
    basename=0x7fffffffe13a "/tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8")
    at src/cabextract.c:483
#5  main (argc=2, argv=0x7fffffffdd58) at src/cabextract.c:412
(gdb) disassemble 
Dump of assembler code for function __gconv:
   0x00007ffff7a170b0 <+0>: cmp    rdi,0xffffffffffffffff
   0x00007ffff7a170b4 <+4>: je     0x7ffff7a171e8 <__gconv+312>
   0x00007ffff7a170ba <+10>:    push   r15
   0x00007ffff7a170bc <+12>:    push   r14
   0x00007ffff7a170be <+14>:    mov    r15,rdi
   0x00007ffff7a170c1 <+17>:    push   r13
   0x00007ffff7a170c3 <+19>:    push   r12
   0x00007ffff7a170c5 <+21>:    mov    r14,r9
   0x00007ffff7a170c8 <+24>:    push   rbp
   0x00007ffff7a170c9 <+25>:    push   rbx
   0x00007ffff7a170ca <+26>:    mov    r12,rdx
   0x00007ffff7a170cd <+29>:    mov    rbp,rsi
   0x00007ffff7a170d0 <+32>:    sub    rsp,0x28
=> 0x00007ffff7a170d4 <+36>:    mov    rax,QWORD PTR [rdi]
(gdb) info registers
rax            0x0  0
rbx            0x555555765c30   93824994401328
rcx            0x0  0
rdx            0x0  0
rsi            0x0  0
rdi            0x0  0
rbp            0x0  0x0
rsp            0x7fffffffd9d0   0x7fffffffd9d0
r8             0x0  0
r9             0x7fffffffda30   140737488345648
r10            0xffffffffffffffb0   -80
r11            0x7ffff7dcfc78   140737351842936
r12            0x0  0
r13            0x0  0
r14            0x7fffffffda30   140737488345648
r15            0x0  0
rip            0x7ffff7a170d4   0x7ffff7a170d4 <__gconv+36>

The contained file can be extracted successfully on Windows. I'll email you the offending CAB file.

kyz commented 6 years ago

Please do send me the cabinet, but it can't crash in __gconv() in cabextract 1.6, because the cabextract 1.6 source code doesn't have a convert_filenames() function and doesn't have any support for iconv. That's a feature of the as-yet unreleased cabextract 1.7

$ curl -qO https://www.cabextract.org.uk/cabextract-1.6.tar.gz
$ tar xf cabextract-1.6.tar.gz
$ grep -r convert_filename cabextract-1.6
$ grep -r gconv cabextract-1.6
$ grep -r iconv cabextract-1.6
kyz commented 6 years ago

Thanks for sending me the file. I can confirm it's a problem in the as-yet-not-released cabextract 1.7 with its new support for libiconv, and not a problem in any earlier version of cabextract.

$ for x in cabextract-1.{2..6} ./cabextract; do $x --version; $x -t badcab.cab; done
cabextract version 1.2
Testing cabinet: badcab.cab
  QUTMOE.exe  OK                               77a4948e2781d5d97d5cd3ee9356cc3b

All done, no errors.
cabextract version 1.3
Testing cabinet: badcab.cab
  QUTMOE.exe  OK                               77a4948e2781d5d97d5cd3ee9356cc3b

All done, no errors.
cabextract version 1.4
Testing cabinet: badcab.cab
  QUTMOE.exe  OK                               77a4948e2781d5d97d5cd3ee9356cc3b

All done, no errors.
cabextract version 1.5
Testing cabinet: badcab.cab
  QUTMOE.exe  OK                               77a4948e2781d5d97d5cd3ee9356cc3b

All done, no errors.
cabextract version 1.6
Testing cabinet: badcab.cab
  QUTMOE.exe  OK                               77a4948e2781d5d97d5cd3ee9356cc3b

All done, no errors.
cabextract version 1.7
Segmentation fault (core dumped)

I'll look into this and make sure it's fixed before cabextract 1.7 is released.

recvfrom commented 6 years ago

Ah, ok. The first block of output in my comment above is from cabextract 1.6, and the second block of output is from cabextract 1.7 I built from the master branch. Here is the stack trace from 1.6:

(gdb) bt
#0  0x0000000000401d20 in ?? ()
#1  0x00007ffff78021c1 in __libc_start_main (main=0x4012e0, argc=2, argv=0x7fffffffdda8, 
    init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdd98)
    at ../csu/libc-start.c:308
#2  0x0000000000402889 in ?? ()
kyz commented 6 years ago

Could you include the compilation flags needed to build cabextract 1.6 in order to get it to crash?

I can definitely get a crash in pre-1.7 (just by listing it, not even extracting it), but I can't get any crash in 1.6, and not a peep out of valgrind or asan on Ubuntu 16.04 LTS.

Can you share the compilation flags and patches to vanilla cabextract-1.6 that were needed to get it to crash? I can't make it crash, unlike pre-1.7:

$ make
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o system.o `test -f 'mspack/system.c' || echo './'`mspack/system.c
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o cabd.o `test -f 'mspack/cabd.c' || echo './'`mspack/cabd.c
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o lzxd.o `test -f 'mspack/lzxd.c' || echo './'`mspack/lzxd.c
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o mszipd.o `test -f 'mspack/mszipd.c' || echo './'`mspack/mszipd.c
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o qtmd.o `test -f 'mspack/qtmd.c' || echo './'`mspack/qtmd.c
rm -f libmspack.a
ar cru libmspack.a system.o cabd.o lzxd.o mszipd.o qtmd.o 
ar: `u' modifier ignored since `D' is the default (see `U')
ranlib libmspack.a
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o cabextract.o `test -f 'src/cabextract.c' || echo './'`src/cabextract.c
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o md5.o md5.c
gcc  -g -O2 -fsanitize=address   -o cabextract cabextract.o md5.o libmspack.a  
gcc -DHAVE_CONFIG_H -I.  -I./mspack -DMSPACK_NO_DEFAULT_SYSTEM   -g -O2 -fsanitize=address -c -o cabinfo.o `test -f 'src/cabinfo.c' || echo './'`src/cabinfo.c
gcc  -g -O2 -fsanitize=address   -o src/cabinfo cabinfo.o  
make[1]: Leaving directory '/home/kyz/cabextract-1.6'
$ ./cabextract badcab.cab 
Extracting cabinet: badcab.cab
  extracting QUTMOE.exe

All done, no errors.
$ valgrind ./cabextract badcab.cab 
==3493== Memcheck, a memory error detector
==3493== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==3493== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==3493== Command: ./cabextract badcab.cab
==3493== 
==3493== Warning: set address range perms: large range [0x27000000, 0x38000000) (defined)
==3493==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.
==3493== 
==3493== HEAP SUMMARY:
==3493==     in use at exit: 0 bytes in 0 blocks
==3493==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==3493== 
==3493== All heap blocks were freed -- no leaks are possible
==3493== 
==3493== For counts of detected and suppressed errors, rerun with: -v
==3493== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
recvfrom commented 6 years ago

Interesting... I didn't build cabextract 1.6 from source, I installed it from the Ubuntu 17.10 aptitude repo with apt-get install cabextract. As best I can tell, the build log for the package is here:

https://launchpadlibrarian.net/205749269/buildlog_ubuntu-wily-amd64.cabextract_1.6-1_BUILDING.txt.gz

From there:

./configure CPPFLAGS="`dpkg-buildflags --get CPPFLAGS`" CFLAGS="`dpkg-buildflags --get CFLAGS`" LDFLAGS="`dpkg-buildflags --get LDFLAGS`" --prefix=/build/buildd/cabextract-1.6/debian/cabextract/usr --mandir=/build/buildd/cabextract-1.6/debian/cabextract/usr/share/man --infodir=/build/buildd/cabextract-1.6/debian/cabextract/usr/share/info --with-external-libmspack=yes

and it looks like they apt-get install libmspack0 version 0.5-1 as part of that build

make[2]: Entering directory '/build/buildd/cabextract-1.6'
gcc -DHAVE_CONFIG_H -I.   -D_FORTIFY_SOURCE=2  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -c -o cabextract.o `test -f 'src/cabextract.c' || echo './'`src/cabextract.c
gcc -DHAVE_CONFIG_H -I.   -D_FORTIFY_SOURCE=2  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -c -o md5.o md5.c
gcc  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security  -Wl,-Bsymbolic-functions -Wl,-z,relro -o cabextract cabextract.o md5.o -lmspack 
gcc -DHAVE_CONFIG_H -I.   -D_FORTIFY_SOURCE=2  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -c -o cabinfo.o `test -f 'src/cabinfo.c' || echo './'`src/cabinfo.c
src/cabinfo.c: In function 'main':
src/cabinfo.c:172:3: warning: format '%lld' expects argument of type 'long long int', but argument 3 has type 'off_t' [-Wformat=]
   printf("Examining file \"%s\" (%" FL " bytes)...\n", filename, filelen);
   ^
src/cabinfo.c: In function 'search':
src/cabinfo.c:245:4: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type 'off_t' [-Wformat=]
    printf("Found cabinet header at offset %" FL "\n", caboff);
    ^
src/cabinfo.c: In function 'getinfo':
src/cabinfo.c:305:3: warning: format '%lld' expects argument of type 'long long int', but argument 4 has type 'off_t' [-Wformat=]
   );
   ^
src/cabinfo.c:335:5: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type '__off_t' [-Wformat=]
     header_res);
     ^
src/cabinfo.c:392:5: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type 'off_t' [-Wformat=]
     );
     ^
src/cabinfo.c:392:5: warning: format '%lld' expects argument of type 'long long int', but argument 3 has type 'off_t' [-Wformat=]
src/cabinfo.c:398:7: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type '__off_t' [-Wformat=]
       folder_res);
       ^
src/cabinfo.c:465:5: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type 'off_t' [-Wformat=]
     );
     ^
src/cabinfo.c:478:5: warning: format '%lld' expects argument of type 'long long int', but argument 3 has type 'off_t' [-Wformat=]
     ((x > (32768+6144)) || (y > 32768)) ? " INVALID" : "");
     ^
gcc  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security  -Wl,-Bsymbolic-functions -Wl,-z,relro -o src/cabinfo cabinfo.o  
make[2]: Leaving directory '/build/buildd/cabextract-1.6'
kyz commented 6 years ago

I've just pushed a fix for cabextract pre-1.7; cabextract would segfault on every cabinet in the world if the parameter --encoding wasn't included. Hopefully that's cleared up.

However, you are probably still getting a segfault just because of this file and not because of that bug. I now can't reproduce the bug on any version of cabextract.

My setup is

What environment are you using?

kyz commented 6 years ago

I can say that libmspack0 0.5-1 is out of date. Now, that version number is simply what's listed in the build and could be updated independently of cabextract, but if that's what's installed on your system (it shouldn't be, you said your system was Ubuntu 17.10), it should be upgraded. At minimum libmspack0 0.6-1 should be installed as it fixes two known CVEs.

So this seems to be the environment:

$ ./configure --with-external-libmspack && make CFLAGS='-g -O2 -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wformat -Werror=format-security'

With this setup (and the system libmspack0 at version 0.5-1ubuntu0.16.04.1), I still can't get a segfault from cabextract.

recvfrom commented 6 years ago

You are right, that is likely the issue - I have an older libmspack installed that is being used:

$ ldd /usr/bin/cabextract
    linux-vdso.so.1 =>  (0x00007ffed07fa000)
    libmspack.so.0 => /usr/local/lib/libmspack.so.0 (0x00007f98226a2000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f98222c2000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f98228b6000)

$ sudo apt-get install libmspack0
...
libmspack0 is already the newest version (0.6-3).
...

$ find /usr/ -name libmspack*
/usr/local/lib/libmspack.so.0.1.0
/usr/local/lib/libmspack.so
...
/usr/local/lib/libmspack.so.0
...
/usr/lib/x86_64-linux-gnu/libmspack.so.0.1.0
/usr/lib/x86_64-linux-gnu/libmspack.so
...
/usr/lib/x86_64-linux-gnu/libmspack.so.0
...

$ export LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu/

$ ldd /usr/bin/cabextract 
    linux-vdso.so.1 =>  (0x00007ffd94a8a000)
    libmspack.so.0 => /usr/lib/x86_64-linux-gnu/libmspack.so.0 (0x00007f3e66ee9000)
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f3e66b09000)
    /lib64/ld-linux-x86-64.so.2 (0x00007f3e670fc000)

$ cabextract /tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8 
Extracting cabinet: /tmp/97f3c838aa94567ca24d15f810a6d1c116d7b971cf2df82188ac4f9ea0a0d9a8
  extracting QUTMOE.exe

All done, no errors.

Sorry about the confusion - the unrelated issue in the master branch led me to initially believe that the issue was more than just an environmental one. Anyway, thanks for your help!