search

pfalcon / uzlib

Radically unbloated DEFLATE/zlib/gzip compression/decompression library. Can decompress any gzip/zlib data, and offers simplified compressor which produces gzip-compatible output, while requiring much less resources (and providing less compression ratio of course).
Other
306 stars 83 forks source link

Possible illegal memory access #5

Closed prusnak closed 6 years ago

prusnak commented 7 years ago

https://github.com/pfalcon/uzlib/blob/master/src/tinflate.c#L298

num is unsigned int

If num == 0, this could lead illegal memory access.

Suggestion: adding check whether num >= 1 and skipping the case if that's not the case.

pfalcon commented 7 years ago

@prusnak: I invited the other guy who talked about fuzzy testing uPy to test uzlib and re1.5 instead. I invite you to do the same and report actual plausible errors.

That specific code is inherited from upstream, and it could be that algorithm doesn't allow that branch to be taken with n == 0. Feel free to provide actual testcase to show the problem. Otherwise, thanks for the report, I'll be looking at the detail into it when I have time. Just adding checks for everything everywhere without proof will make it slow.

prusnak commented 7 years ago

I thought this was the upstream for uzlib. I agree that the code might not be reachable when providing a valid input. But that's not what attackers do. :-)

pfalcon commented 7 years ago

This is the upstream for uzlib. Please read README to understand where it comes from.

prusnak commented 7 years ago

Ah, I thought uzlib diverged from tinf to the point where it is almost a separate project. Sorry for the noise.

pfalcon commented 7 years ago

Provide invalid input which makes that branch taken with invalid state of variables, that's the whole point, and what fuzz testing does automate, no? Alternatively, if you reviewed the algorithm and can provide Dijkstra- or Knuth- style proof that n==0 is possible in that branch, that works too ;-). If not, /me will review it (no Dijkstra-style proof however, but i'll do my best).

pfalcon commented 7 years ago

it's a separate, with majority of decoding code still from tinf.

pfalcon commented 6 years ago

This issue is finally fixed in 66d3fb43a474125c86865b649eed04d0d83fa942, following contribution on fuzztest corpus in https://github.com/pfalcon/uzlib/issues/9