Bug#871263: libmspack: CVE-2017-6419
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
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
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
+ 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
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