kyz/libmspack

Warnings from compilers/analyzers

DmitryGlavatskikh opened this issue · 2 comments

What is your politics about compilers or code analyzeres warnings?
I know this is mostly endless fight... But who knows...
While investigating possibility of using your famous libmspack library in our commercial project I found huge ammont of warning from different compilers and static code analyzers.
Most of them is just some sort of common signed/unsigned mismatch. Some are false positive for shure but some other looks for me like really potential bugs. Hard to say exactly bcoz I don't familiar with your library interanals.
Pretty shure I should not put them here in public for common reason but just one example:
define TOLOWER(x) (((x)<0||(x)>256)?(x):mspack_tolower_map[(x)])
off-by-one static const array overrun here for 0x100 utf-8 character.

kyz commented

My politics is that finding and fixing bugs is fine, and static analysis can help with that. But rewriting correct code in a specific way to make a static analyzer happy is a waste of time at best, and introduces new bugs at worst.

With that in mind, I'd be happy for you to list anything you think is a genuine bug. If you think it's an exploitable security bug, please email me directly, but otherwise it can just be listed here.

I agree with you there is a one byte overread when calling chmd->fast_find() where one of the filenames in the index contains character U+0100, and libmspack is compiled with neither towlower() or tolower(). I'll fix the off-by-one error. Thanks for reporting it.

kyz commented

Thanks again for reporting the one byte overread, it was fixed in commit 4fd9cca and is now part of libmspack release 0.7alpha