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 <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.
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