[Pkg-clamav-devel] Bug#773659: cabextract: null pointer dereference on a crafted CAB

[Sebastian Andrzej Siewior]

* Jakub Wilk | 2014-12-21 18:35:36 [+0100]:

>Package: cabextract
>Version: 1.4-4+b1
>Usertags: afl
>
>cabextract crashes (trying to dereference null pointed) on the attached
>crafted CAB file:
Jakub, please fill future bugs against libmspack and CC the clamav team. I
am interrested in getting those fixed before they spread since they can
affect clamav.
I'm going to clone this one against libnspack and mark it as fixed in
cabextract after the library switch.

>$ gpg -d nullderef.cab.asc > nullderef.cab
>$ cabextract -t nullderef.cab
>nullderef.cab: WARNING; possible 1626 extra bytes at end of file.
>Testing cabinet: nullderef.cab
>   failed (error in CAB data format)
>   failed (Success)
> E  failed (error in CAB data format)
>Segmentation fault
>
>
>Backtrace:
>#0  0x00000000 in ?? ()
>#1  0x0804e094 in cabd_extract (base=0x805b008, file=0x8063600, filename=0x8056643 "test") at mspack/cabd.c:1068
>#2  0x080493b4 in process_cabinet (basename=0xffffd9b8 "nullderef.cab") at src/cabextract.c:467
>#3  0x08048fc4 in main (argc=3, argv=0xffffd804) at src/cabextract.c:350

The ->search callback of the mspack library finds two cab files within
the one you attached. The internal structure gets real funny. afl
managed to create a .cab file which contains a valid file, followed by
one which contains an invalid compression which removes the
decompression callback. And then mspack thinks that the following file
belongs to the previous folder and therefore the decompression callback
is not updated but have none assigned and the NULL pointer is invoked. I
am not yet sure where this should be fixed but the easy fix is to check
the null pointer cabd_extract() before the invocation.
I will try to check if it is possible to catch this earlier…

The good news is that clamav is not affected by this since it seems not
to trigger if the ->search callback is not invoked. Also we stop
scanning once an invalid file is found within the archive. Not sure if
this is good news…

Sebastian