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

[Sebastian Andrzej Siewior]

On 2017-08-15 05:55:49 [+0900], Marc Dequènes (Duck) wrote:
> Quack,
Hi,

> I was at DebConf in Canada, so I was busy meeting people :-).
> It should be done before or after flying back home.

No worries. We got the two CVEs sorted out and a release in the
meantime. I see an unstable upload almost made it (B-D doxygen missing).
And we need a security upload.
> \_o<
> 

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

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

[Duck]

Quack,

On 08/07/2017 04:22 AM, Sebastian Andrzej Siewior wrote:

> Marc do plan you upload something to unstable/security soon, wait for a
> new release or would you prefer someone else to NMU it with this
> change?

I was at DebConf in Canada, so I was busy meeting people :-).
It should be done before or after flying back home.

\_o<



signature.asc
Description: OpenPGP digital signature
___
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

[Sebastian Andrzej Siewior]

On 2017-08-06 10:22:11 [+0100], Stuart Caie wrote:
> Commited a fix: 
> https://github.com/kyz/libmspack/commit/17038206fcc384dcee6dd9e3a75f08fd3ddc6a38
> 
> I'll put out a release in the near future.

thank you Stuart.
Marc do plan you upload something to unstable/security soon, wait for a
new release or would you prefer someone else to NMU it with this
change?

> Regards
> Stuart

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

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

[Sebastian Andrzej Siewior]

On 2017-07-23 16:52:16 [+0100], Stuart Caie wrote:
> Hello,
Hi Stuart,

> https://github.com/kyz/libmspack/commit/3e3436af6010ac245d7a390c6798e2b81ce09191
> > 2015-05-10  Stuart Caie 
> > * cabd_read_string(): correct rejection of empty strings. Thanks to
> > Hanno Böck for finding the issue and providing a sample file.
> 
> I had a philosophical discussion with Hanno Böck about it, I wasn't
> persuaded that it's a real vulnerability. If you craft a CAB file with an
> empty CAB string, one byte will be overread. You can't make it over-read an
> arbitrary number of bytes, just the empty string -> 1 byte overread.
> 
> This report says "and application crash" -- I still have no evidence this is
> true (unless you've instrumented your code to monitor all overreads and
> deliberately crash yourself when you see one). If you want me to release
> libmspack to address a CVE created for a non-vulnerability, please let me
> know.

let me try to bring some light into it. First clamav fixed the issue via:
  
https://github.com/vrtadmin/clamav-devel/commit/ffa31264a657618a0e40c51c01e4bfc32e244d13
  
https://github.com/vrtadmin/clamav-devel/commit/ada5f94e5cfb04e1ac2a6f383f2184753f475b96

and the read function was crafted by the author of this email and looks
like this:
  
https://sources.debian.net/src/clamav/0.99.2%2Bdfsg-6/libclamav/libmspack.c/#L125

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.

> Regards
> Stuart

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