Re: [Pkg-clamav-devel] Bug#868956: libmspack: CVE-2017-11423

[Stuart Caie]

On 05/08/17 10:36, Stuart Caie wrote:

libmspack is wrong to convert to unsigned without checking for errors first.

When I get to my computer, I'll check all calls to mspack_system 
read/write/seek/tell methods, to be sure this doesn't happen anywhere else.

I checked all the other mspack_system calls, they're handled correctly.

Commited a fix: 
https://github.com/kyz/libmspack/commit/17038206fcc384dcee6dd9e3a75f08fd3ddc6a38


I'll put out a release in the near future.

Before fix, allowing N reads before always failing in cabd_memory.c 
sys->read():

Allow 3 reads -> mspack/cabd.c:528 (cabd_read_string) len=4294967295
Allow 4 reads -> mspack/cabd.c:528 (cabd_read_string) len=193
Allow 5 reads -> mspack/cabd.c:528 (cabd_read_string) len=193 
mspack/cabd.c:528 (cabd_read_string) len=4294967295
Allow 6 reads -> mspack/cabd.c:528 (cabd_read_string) len=193 
mspack/cabd.c:528 (cabd_read_string) len=169


After fix:
Allowing 3 reads -> error caught and no len printed
Allowing 4 reads -> mspack/cabd.c:531 (cabd_read_string) len=193
Allowing 5 reads -> mspack/cabd.c:531 (cabd_read_string) len=193, error 
caught and no len printed
Allowing 6 reads -> mspack/cabd.c:531 (cabd_read_string) len=193 
mspack/cabd.c:531 (cabd_read_string) len=169


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#868956: libmspack: CVE-2017-11423

[Stuart Caie]

On 4 Aug 2017 7:40 am, Sebastian Andrzej Siewior  
wrote:
>
> The way I see it, the problem is that the read functions returns -1 on 
> error and libmspack 
>   https://sources.debian.net/src/libmspack/0.5-1/mspack/cabd.c/#L524 
>
> treats the return code as unsigned integer which makes the error (-1) 
> slightly large. The test files cabd_memory.c and multifh.c also return 
> -1 on error.

Good catch. That's a new bug I hadn't seen before.

mspack_system.read promises to return negative numbers: 
https://www.cabextract.org.uk/libmspack/doc/structmspack__system.html#ac33dcc54409a7d5da9be475b3938101e

libmspack is wrong to convert to unsigned without checking for errors first.

When I get to my computer, I'll check all calls to mspack_system 
read/write/seek/tell methods, to be sure this doesn't happen anywhere else.

I'll put out a fix ASAP, but the good news is this seems tricky to exploit. You 
need to get read() to return an error, not bytes or EOF. The default 
mspack_system uses fread(), so it couldn't be done there just by file contents. 
Custom mspack_systems need to exploitable enough to reach the core bug, so not 
all libmspack usages are vulnerable.

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#773659: cabextract: null pointer dereference on a crafted CAB

[Stuart Caie]

On 16/01/2015 20:29, Sebastian Andrzej Siewior wrote:


Well, it looks like Jakub did not stop yet. Atleast those two do not do
not crash immediately.

- libmspack: off-by-one buffer over-read in mspack/mszipd.c
   https://bugs.debian.org/775498

- libmspack: off-by-one(?) buffer under-read in mspack/lzxd.c
   https://bugs.debian.org/775499

Now fixed in the libmspack repository.

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

[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#773659: cabextract: null pointer dereference on a crafted CAB

[Stuart Caie]

On 11/01/2015 21:15, Sebastian Andrzej Siewior wrote:

On 2015-01-11 16:31:30 [+], Stuart Caie wrote:

This is an accurate summary. There are two cab files found, the second of

Sorry for the inaccurate summary.

No, the summary was accurate :)
Are you also aware of the two recent reports which are .chm based? 
https://bugs.debian.org/774725 https://bugs.debian.org/774726
I am now. I've sense-checked the patch for 774726 and it passes my test 
suite, so it's now committed to the repository. I'm doing the same for 774725.

In total Jakub reported four issues.

I thank him for it! libmspack is now more robust because of his work.

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