Bug#914794: libmspack fails tests on big endian architectures (s390x, mips)

[Stuart Caie]

On 04/03/2019 05:00, Marc Dequènes (duck) wrote:

Quack,

On 2019-03-04 10:40, Stuart Caie wrote:

I've released libmspack 0.10 and cabextract 1.9.1. They contain only 
fixes.


Thanks a lot for being so fast.

Unfortunately there is a build problem:


How odd, but yes, I have a system where size_t = unsigned int, so it 
went unnoticed. I've fixed it and tested it on both the 32bit system and 
a 64bit system.


I've released libmspack 0.10.1

Regards
Stuart

Bug#914794: libmspack fails tests on big endian architectures (s390x, mips)

[Stuart Caie]

On 03/03/2019 04:47, Marc Dequènes (duck) wrote:

Quack,

On 2018-12-04 19:02, Stuart Caie wrote:


This is fixed in the repository, it just hasn't been released. I'll
release it in the near future.


I was myself busy and we missed the Debian freeze deadline. Maybe 
there is still some hope to convince the release team for an 
exception, as long as we push fixes only. Any plan to release shortly?


I've released libmspack 0.10 and cabextract 1.9.1. They contain only fixes.

Regards
Stuart

Bug#914794: libmspack fails tests on big endian architectures (s390x, mips)

[Stuart Caie]

On 04/12/2018 05:35, Marc Dequènes (duck) wrote:



libmspack fails tests on big endian architectures (s390x, mips)
This is fixed in the repository, it just hasn't been released. I'll 
release it in the near future.


commit c19e707936947b45cf05bc9aaee68517c6c2aca6
Author: Stuart Caie 
Date:   Thu Nov 8 11:58:45 2018 +

    Add AC_C_BIGENDIAN so md5.c works on big-endian systems without 
manually setting WORDS_BIGENDIAN


commit 80fb91f3680f5acf96fd372b95c9b6205e0e9cbf (tag: v0.9.1alpha)
Author: Stuart Caie 
Date:   Tue Nov 6 11:28:26 2018 +

    Re-release 0.9alpha as 0.9.1alpha with the doc/ directory 
re-included in distribution


Regards
Stuart

Bug#868956: libmspack: CVE-2017-11423

[Stuart Caie]

For your information, libmspack 0.6alpha has now been released.

On 06/08/17 20:22, Sebastian Andrzej Siewior wrote:

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

Bug#871263: libmspack: CVE-2017-6419

[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

[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#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

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

Bug#868956: libmspack: CVE-2017-11423

[Stuart Caie]

Hello,

I have no more infomation than you do. If you can find out who raised 
the issue, please ask them to send me the example of the crafted file,


The bug says "stack-based buffer over-read and application crash" - the 
file 
https://github.com/hackerlib/hackerlib-vul/tree/master/clamav-vul/stack-overflow 
doesn't show an application crash, it shows only the stack-based buffer 
over-read of 1 byte.


I've know about that one-byte buffer over-read, I fixed it in 2015, and 
I haven't yet got around to making a release of libmspack with this fix, 
because I didn't consider it a vulnerability at the time and still don't 
consider it one now.


https://github.com/kyz/libmspack/commit/3e3436af6010ac245d7a390c6798e2b81ce09191

2015-05-10  Stuart Caie <ky...@4u.net>
* 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.


Regards
Stuart

On 23/07/17 16:17, Marc Dequènes (duck) wrote:

Quack,

I added libmspack's upstream author in case he could give a hand.
Here is the bugreport: 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=868956


On 2017-07-20 05:15, Salvatore Bonaccorso wrote:


Unfortunately the upstream bug [1] is locked-down.


Thanks for reporting it. Unfortunately I don't see how I can solve 
this problem. If all information are hidden on a related but not 
upstream bug tracker (which really should have one), if there's no 
patch or new release either, then I'm honestly at a loss.


If I happen to create an account on the ClamAV's bug tracker, would 
you be able to give me access?


Regards.
\_o<

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


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org