On 4 Aug 2017 7:40 am, Sebastian Andrzej Siewior <sebast...@breakpoint.cc> 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
