[Pkg-clamav-devel] Bug#775687: libmspack: CHM decompression: another pointer arithmetic overflow

2015-01-18 Thread Jakub Wilk

Package: libmspack0
Version: 0.4-3
Severity: grave
Tags: security
Usertags: afl

Sorry, it's me again! libmspack crashes on the attached file:

$ gpg -d < crash.chm.asc > crash.chm
$ test/chmd_md5 crash.chm
*** crash.chm
WARNING; contents are corrupt
d41d8cd98f00b204e9800998ecf8427e /#ITBITS
5c9d7ff7a1fdaf9bcb9b3fc78b677972 /#SYSTEM
Segmentation fault

Backtrace:
#0  0x56559ca6 in search_chunk (chm=0x565641a8, chunk=0x565652e8 "PMGL\323\r", 
filename=0x5656098c "::DataSpace/Storage/MSCompressed/ControlData", result=0xd3a4, 
result_end=0xd3a8) at mspack/chmd.c:805
#1  0x5655943b in chmd_fast_find (base=0x56564008, chm=0x565641a8, filename=0x5656098c 
"::DataSpace/Storage/MSCompressed/ControlData", f_ptr=0xd430, f_size=28) at 
mspack/chmd.c:581
#2  0x5655b3c7 in find_sys_file (self=0x56564008, sec=0x565641d8, f_ptr=0x565641e4, 
name=0x5656098c "::DataSpace/Storage/MSCompressed/ControlData") at 
mspack/chmd.c:1304
#3  0x5655aa4a in chmd_init_decomp (self=0x56564008, file=0x56565228) at 
mspack/chmd.c:1075
#4  0x5655a787 in chmd_extract (base=0x56564008, file=0x56565228, filename=0x0) 
at mspack/chmd.c:998
#5  0x56556304 in main (argc=2, argv=0xd848) at test/chmd_md5.c:44


The problem is that the bounds check in mspack/chmd.c:788:

   if (name_len > end - p) goto chunk_end;

doesn't work on 32-bit systems if "p" is already bigger than "end" (as 
is the case for crash.chm), because negative "end - p" gets 
automatically converted to unsigned int.


A quick and dirty fix would be to rewrite the bounds checks as

   if (p > end || name_len > end - p) goto chunk_end;

but it'd be better to fix the thing that sets "p" to a value past the 
"end".



-- System Information:
Debian Release: 8.0
 APT prefers unstable
 APT policy: (990, 'unstable'), (500, 'experimental')
Architecture: i386 (x86_64)
Foreign Architectures: amd64

Kernel: Linux 3.2.0-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=C, LC_CTYPE=pl_PL.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages libmspack0 depends on:
ii  libc6  2.19-13
ii  multiarch-support  2.19-13

--
Jakub Wilk
-BEGIN PGP MESSAGE-
Version: GnuPG v1

owFbK/A+iTO5KLE4Qy85Izdk95sJniHBbswMDAwJQMwIxGnzOZdxsjAwCPxlrFlV
fUFwHg/DgpNKz94IovFB6kFAAkpXQOkQAQh9Bkr/Y4TQLsoMKABobwBIKgSIuUAC
QPVMUDf8BwKYOhAbJAZyU9Ykplg9xQuCc39C3BAClYeBAF93n8u8CH1gvfpADoe+
sqdLhIdLEGOjzwIwL8QJ6ACQsfrKwSFBnn7uwYwLaxiBMsGRwSGuvgzsbAuqgLwQ
/wBPZ6CUjwCQExrkA1TLuDBDBMIJcfJhXBjDw62fmZeSWqGXUZKbw8jQ6CNiZeWS
WJIYXJCYnKrvl5ib6pNZXMLAYKOBLB5ckl+UmJ6q7xvsnJ9bUJRaXJyaou+cn1eS
mlfCsIihyUOHKNVF+TkgVQxZMpoE1QOl8jzz0vIZkjj0CSoOKUrMK07LL8rVB7ve
Ri2eBC3V5m7ORhaWJga6li7GhrqGhi5AlpORua6BgaOBs6Whq6WzuXOtvmdecUli
XnIq0Nyz9AXngZhhFIyCUTAKRsEoGAWjYBSMglHwa6AdMApGAYlgNWMXgzWDAIMc
AxMDD0MoQx5DMkM+Qy5DAUMRAwA=
=s4xD
-END PGP MESSAGE-
___
Pkg-clamav-devel mailing list
Pkg-clamav-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-clamav-devel

Re: [Pkg-clamav-devel] Bug#775687: libmspack: CHM decompression: another pointer arithmetic overflow

2015-01-18 Thread Sebastian Andrzej Siewior
On 2015-01-18 18:59:33 [+0100], Jakub Wilk wrote:
> Sorry, it's me again! libmspack crashes on the attached file:
As I've seen your ubsan reports, I assumed you were done. Wrong this
was.

> $ gpg -d < crash.chm.asc > crash.chm
> $ test/chmd_md5 crash.chm
> *** crash.chm
> 
> but it'd be better to fix the thing that sets "p" to a value past the "end".

So something like the patch attached then?. But this should be
double-checked in case we properly come to end and don't continue
using p anymore. But not today…

Sebastian
>From d78acf012a47fb4e868a87ef738a947ad3aa7ea3 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior 
Date: Sun, 18 Jan 2015 22:47:45 +0100
Subject: [PATCH] mspack/chmd: check p > end also after we left

Jakub's AFL produced a case where "p" reads the name_len just just at the
end of the buffer. The macro increases p and makes it bigger than end.
The following oversize check fails on 32bit and bom. The patch ensures
that we check p vs end even after we legally leave the loop.

Signed-off-by: Sebastian Andrzej Siewior 
---
 mspack/chmd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mspack/chmd.c b/mspack/chmd.c
index 4caff45..22661f2 100644
--- a/mspack/chmd.c
+++ b/mspack/chmd.c
@@ -257,6 +257,7 @@ static const unsigned char guids[32] = {
 	if (p > end) goto chunk_end;		\
 	(var) = ((var) << 7) | (*p & 0x7F);	\
 } while (*p++ & 0x80);			\
+if (p > end) goto chunk_end;		\
 } while (0)
 
 static int chmd_read_headers(struct mspack_system *sys, struct mspack_file *fh,
-- 
2.1.3

___
Pkg-clamav-devel mailing list
Pkg-clamav-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-clamav-devel

Re: [Pkg-clamav-devel] Bug#775687: libmspack: CHM decompression: another pointer arithmetic overflow

2015-01-18 Thread Stuart Caie

On 18/01/2015 22:00, Sebastian Andrzej Siewior wrote:

On 2015-01-18 18:59:33 [+0100], Jakub Wilk wrote:

Sorry, it's me again! libmspack crashes on the attached file:

As I've seen your ubsan reports, I assumed you were done. Wrong this
was.


$ gpg -d < crash.chm.asc > crash.chm
$ test/chmd_md5 crash.chm
*** crash.chm

but it'd be better to fix the thing that sets "p" to a value past the "end".

So something like the patch attached then?. But this should be
double-checked in case we properly come to end and don't continue
using p anymore. But not today…


I made this change instead.

@@ -254,7 +254,7 @@
 #define READ_ENCINT(var) do {  \
 (var) = 0; \
 do {   \
-   if (p > end) goto chunk_end;\
+   if (p >= end) goto chunk_end;   \
(var) = ((var) << 7) | (*p & 0x7F); \
 } while (*p++ & 0x80); \
 } while (0)

Regards
Stuart

___
Pkg-clamav-devel mailing list
Pkg-clamav-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-clamav-devel


Re: [Pkg-clamav-devel] Bug#775687: libmspack: CHM decompression: another pointer arithmetic overflow

2015-01-29 Thread Sebastian Andrzej Siewior
0.5alpha has been just released [0] with this issue fixed. If you
package that one you get rid of all currently known bugs :)

[0] http://www.cabextract.org.uk/libmspack/libmspack-0.5alpha.tar.gz

Sebastian

___
Pkg-clamav-devel mailing list
Pkg-clamav-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-clamav-devel