Bug#871263: libmspack: CVE-2017-6419

2017-08-13 Thread Stuart Caie



On 12/08/17 20:40, Sebastian Andrzej Siewior wrote:

On 2017-08-12 00:42:06 [+0100], Stuart Caie wrote:

On 11/08/17 19:07, Sebastian Andrzej Siewior wrote:

[0] https://security-tracker.debian.org/tracker/CVE-2017-6419
  https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6419
[1] 
https://github.com/vrtadmin/clamav-devel/commit/a83773682e856ad6529ba6db8d1792e6d515d7f1

Stuart, is this enough information or do you need more?

I'm interested in how the fix is to add a check to see if
window_posn+this_run wraps the window, immediately below a comment that
expressly states that won't happen, with the reasoning: this_run has already
been clamped to ensure it does not wrap a frame, and frames don't wrap
windows.

If this is incorrect reasoning, what part of the reasoning is wrong? Is
this_run somehow not being clamped to <=FRAME_SIZE through some code path?
If so, the fix is to clamp it. Is window size not a multiple of frame size?
If so, something is very wrong.

The CVE links the following link:
   
https://github.com/varsleak/varsleak-vul/blob/master/clamav-vul/heap-overflow/clamav_chm_crash.md
and that folder contains also
   
https://github.com/varsleak/varsleak-vul/raw/master/clamav-vul/heap-overflow/clamav.crash.chm

and clamav segfaults on that one. Could you please check?


OK, so now I know why.

mspack/chmd.c:1191 (read_reset_table) can't read reset table
mspack/lzxd.c:327 (lzxd_init) output_length=-884758871994
mspack/lzxd.c:336 (lzxd_init) reset_interval=16386
mspack/lzxd.c:467 (lzxd_decompress) length=-884758871994
mspack/lzxd.c:468 (lzxd_decompress) offset=0
mspack/lzxd.c:469 (lzxd_decompress) length-offset=-884758871994
mspack/lzxd.c:470 (lzxd_decompress) frame_size=4390982
mspack/lzxd.c:474 (lzxd_decompress) bytes_todo=4390982
mspack/lzxd.c:539 (lzxd_decompress) this_run=4390982
mspack/lzxd.c:777 (lzxd_decompress) uncompressed: window_posn=0, 
this_run=4390982, window_size=65536


The root cause is that chmd.c initialises the lzx stream with an 
incorrect output_length.


We decide how large the LZX frame will be. It'll either be 32k, or, if 
we know how far we are along in the output stream, and we know there is 
less than 32k left to decode, this must be the final frame, which is 
allowed to be less than 32k:


frame_size = LZX_FRAME_SIZE;
if (lzx->length && (lzx->length - lzx->offset) < (off_t)frame_size) {
  frame_size = lzx->length - lzx->offset;
}

(lzx->length - lzx->offset) should always be a positive number.. but 
only if length is positive!


With negative output_length, it fails this test, and sets frame_size to 
any value (the bottom 32 bits of output_length).


Complete control of frame_size could cause trouble in lots of places, 
not just at the uncompressed block copy.


So, the right thing to do is to guard against negative output_length. 
I've done this, and additionally added checking to chmd read_spaninfo() 
to place the blame where it lies:


https://github.com/kyz/libmspack/commit/6139a0b9e93fcb7fcf423e56aa825bc869e02229

The example file is now rejected as invalid data:

mspack/chmd.c:1191 (read_reset_table) can't read reset table
mspack/chmd.c:1276 (read_spaninfo) output length is invalid
clamav.crash.chm: extract error on "/#IDXHDR": error in data format


Yeah. The problem is probably that the reporter did not forward the report to
the upstream project (you) but only to ClamAV. And ClamAV itself isn't very
good at handling such things. I noticed this by chance while checking Debian's
bug report on the other CVE and I assumed it would be best to let you know :)

Thanks very much, I appreciate it.

Regards
Stuart



Bug#871263: libmspack: CVE-2017-6419

2017-08-12 Thread Sebastian Andrzej Siewior
On 2017-08-12 00:42:06 [+0100], Stuart Caie wrote:
> On 11/08/17 19:07, Sebastian Andrzej Siewior wrote:
> > > [0] https://security-tracker.debian.org/tracker/CVE-2017-6419
> > >  https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6419
> > > [1] 
> > > https://github.com/vrtadmin/clamav-devel/commit/a83773682e856ad6529ba6db8d1792e6d515d7f1
> > Stuart, is this enough information or do you need more?
> I'm interested in how the fix is to add a check to see if
> window_posn+this_run wraps the window, immediately below a comment that
> expressly states that won't happen, with the reasoning: this_run has already
> been clamped to ensure it does not wrap a frame, and frames don't wrap
> windows.
> 
> If this is incorrect reasoning, what part of the reasoning is wrong? Is
> this_run somehow not being clamped to <=FRAME_SIZE through some code path?
> If so, the fix is to clamp it. Is window size not a multiple of frame size?
> If so, something is very wrong.

The CVE links the following link:
  
https://github.com/varsleak/varsleak-vul/blob/master/clamav-vul/heap-overflow/clamav_chm_crash.md
and that folder contains also
  
https://github.com/varsleak/varsleak-vul/raw/master/clamav-vul/heap-overflow/clamav.crash.chm

and clamav segfaults on that one. Could you please check?

> I'd be interested in seeing an example file that gets to this condition.
> 
> Also, if ClamAV made a change five months ago, and they're confident it's a
> bug in libmspack why am I only finding out today?

Yeah. The problem is probably that the reporter did not forward the report to
the upstream project (you) but only to ClamAV. And ClamAV itself isn't very
good at handling such things. I noticed this by chance while checking Debian's
bug report on the other CVE and I assumed it would be best to let you know :)

> Regards
> Stuart

Sebastian



Bug#871263: libmspack: CVE-2017-6419

2017-08-11 Thread Stuart Caie

On 11/08/17 19:07, Sebastian Andrzej Siewior wrote:

[0] https://security-tracker.debian.org/tracker/CVE-2017-6419
 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6419
[1] 
https://github.com/vrtadmin/clamav-devel/commit/a83773682e856ad6529ba6db8d1792e6d515d7f1

Stuart, is this enough information or do you need more?
I'm interested in how the fix is to add a check to see if 
window_posn+this_run wraps the window, immediately below a comment that 
expressly states that won't happen, with the reasoning: this_run has 
already been clamped to ensure it does not wrap a frame, and frames 
don't wrap windows.


If this is incorrect reasoning, what part of the reasoning is wrong? Is 
this_run somehow not being clamped to <=FRAME_SIZE through some code 
path? If so, the fix is to clamp it. Is window size not a multiple of 
frame size? If so, something is very wrong.


I'd be interested in seeing an example file that gets to this condition.

Also, if ClamAV made a change five months ago, and they're confident 
it's a bug in libmspack why am I only finding out today?


Regards
Stuart



Bug#871263: libmspack: CVE-2017-6419

2017-08-11 Thread Sebastian Andrzej Siewior
+ Stuart

On 2017-08-07 15:21:48 [+0200], Salvatore Bonaccorso wrote:
> Source: libmspack
> Version: 0.5-1
> Severity: grave
> Tags: security upstream
> 
> Hi,
> 
> the following vulnerability was published for libmspack.
> 
> CVE-2017-6419[0]:
> | mspack/lzxd.c in libmspack 0.5alpha, as used in ClamAV 0.99.2, allows
> | remote attackers to cause a denial of service (heap-based buffer
> | overflow and application crash) or possibly have unspecified other
> | impact via a crafted CHM file.
> 
> It was fixed in ClamAV already at [1].
> 
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
> 
> For further information see:
> 
> [0] https://security-tracker.debian.org/tracker/CVE-2017-6419
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6419
> [1] 
> https://github.com/vrtadmin/clamav-devel/commit/a83773682e856ad6529ba6db8d1792e6d515d7f1

Stuart, is this enough information or do you need more?

> Regards,
> Salvatore

Sebastian



Bug#871263: libmspack: CVE-2017-6419

2017-08-07 Thread Salvatore Bonaccorso
Source: libmspack
Version: 0.5-1
Severity: grave
Tags: security upstream

Hi,

the following vulnerability was published for libmspack.

CVE-2017-6419[0]:
| mspack/lzxd.c in libmspack 0.5alpha, as used in ClamAV 0.99.2, allows
| remote attackers to cause a denial of service (heap-based buffer
| overflow and application crash) or possibly have unspecified other
| impact via a crafted CHM file.

It was fixed in ClamAV already at [1].

If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2017-6419
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-6419
[1] 
https://github.com/vrtadmin/clamav-devel/commit/a83773682e856ad6529ba6db8d1792e6d515d7f1

Regards,
Salvatore