On 12/12/2014 05:00, Eric Sharkey wrote:
On Thu, Dec 11, 2014 at 6:47 PM, Jakub Wilk <jw...@debian.org> wrote:
Package: cabextract
Version: 1.4-4+b1
Severity: minor
Usertags: afl
The attached file makes cabextract hang forever (or at least for two
minutes, after which I lost my patience :-P).
This bug was found using American fuzzy lop:
http://lcamtuf.coredump.cx/afl/
It's definitely an infinite loop. It gets caught in qtmd_decompress()
and never gets out of the loop on line 290.
The problem seems to be on this line:
/* decode more, up to the number of bytes needed, the frame boundary,
* or the window boundary, whichever comes first */
frame_end = window_posn + (out_bytes - (qtm->o_end - qtm->o_ptr));
out_bytes is an off_t (8 bytes) but frame_end is an unsigned int (4
bytes) and it overflows.
The cab file attached to the bug has a file with a 'length' field of
4294967231 and 'offset' of 255, which when added together requests exactly
2^32 bytes of data to be unpacked. When computed using or cast to a 32-bit
int / unsigned int, this is zero. Getting past "more than zero" tests by
being a 64-bit off_t leads to the infinite loop when cast down to a 32-bit
unsigned int.
The CAB file format itself doesn't support folder offsets/file lengths >2^31
(*). I think it makes sense to do better input validation and reject invalid
sizes, than try to write decompression code that processes invalid sizes.
While changing variables to off_t fixes a bug that appears with 64-bit
off_t, I'm not sure it doesn't introduce bugs on systems with 32-bit off_t.
Two unsigned ints <2^31 added together fit in an unsigned int but not a
32-bit off_t.
Below the change I intend to apply for this. I'll release new versions of
libmspack and cabextract in the near future, once I've fixed the other bugs
that have recently been found.
Regards
Stuart
Index: mspack/cabd.c
===================================================================
--- mspack/cabd.c (revision 198)
+++ mspack/cabd.c (working copy)
@@ -445,6 +445,19 @@
file->attribs = EndGetI16(&buf[cffile_Attribs]);
file->offset = EndGetI32(&buf[cffile_FolderOffset]);
+ /* check that neither offset, length nor offset+length exceed the max
+ * length of a folder. offset and length are tested first. If they're both
+ * less than CAB_LENGTHMAX, which is less than 2^31, then their sum must
+ * be less than 2^32, so they do not overflow an unsigned int comparison.
+ */
+ if (file->offset > CAB_LENGTHMAX ||
+ file->length > CAB_LENGTHMAX ||
+ (file->offset + file->length) > CAB_LENGTHMAX)
+ {
+ D(("invalid offset, length or offset+length"))
+ return MSPACK_ERR_DATAFORMAT;
+ }
+
/* set folder pointer */
x = EndGetI16(&buf[cffile_FolderIndex]);
if (x < cffileCONTINUED_FROM_PREV) {
@@ -937,6 +950,12 @@
return 0;
}
+ /* check there are not too many data blocks after merging */
+ if ((lfol->base.num_blocks + rfol->base.num_blocks) > CAB_FOLDERMAX) {
+ D(("folder merge: too many data blocks in merged folders"))
+ return 0;
+ }
+
if (!(lfi = lfol->merge_next) || !(rfi = rfol->merge_prev)) {
D(("folder merge: one cabinet has no files to merge"))
return 0;
Index: mspack/cab.h
===================================================================
--- mspack/cab.h (revision 198)
+++ mspack/cab.h (working copy)
@@ -70,6 +70,13 @@
#define CAB_BLOCKMAX (32768)
#define CAB_INPUTMAX (CAB_BLOCKMAX+6144)
+/* There are no more than 65535 data blocks per folder, so a folder cannot
+ * be more than 32768*65535 bytes in length. As files cannot span more than
+ * one folder, this is also their max offset, length and offset+length limit.
+ */
+#define CAB_FOLDERMAX (65535)
+#define CAB_LENGTHMAX (CAB_BLOCKMAX * CAB_FOLDERMAX)
+
/* CAB compression definitions */
(*): The maximum size of any cab folder is 65535 datablocks, unless you take
the generous interpretation that two folders are allowed to merge to become
a >65535 datablock folder. I haven't seen any such cab files in the wild,
but I have seen MS employees bemoan their file format's lack of support for
>2GB files, so I'm taking it as read that 65535 blocks is the upper limit
for one folder. Blocks are <=32768 bytes each when unpacked, so the maximum
unpacked folder size is 65535*32768 = 2147450880 bytes. Files (in the cab
file) can't span more than one folder, so the maximum offset+length of any
file is also 2147450880 bytes.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
