[llvm-dev] Overlapping memcpy

[Stephen Canon via llvm-dev]

> On Dec 8, 2015, at 1:28 AM, Maciej Adamczyk via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
>> The lack of such error checking is one of the big reasons that libraries like
>> libjpeg, libpng, and so on have been a huge source of vulnerabilities in web
>> browsers for the last couple of decades.  It sounds like your friend has
>> already added a security hole to his library, please discourage him from
>> adding any more.
> 
> So far he has turned to memmove. He surely doesn't want the code working incorrectly and with unknowns he decided to play it safe. But we (me especially) wonder if it really is an issue as we fail to find anything that compiler or stdlib could exploit to make the code misbehave (as corrupting the buffer is OK in this case).

I can pretty much guarantee that "corrupting the buffer” is very much not OK.  Even if that’s the contract that your API provides, there have been numerous CVEs caused by compressors doing exactly this, and either copying sensitive data from a wild pointer (via bogus match distance) or allowing uninitialized memory to pass through (via zero “match distance”), again, possibly leaking sensitive data, or otherwise defeating security checks.  It just takes one higher-level piece of code to not handle the error returned by the compressor correctly and a CVE is born.  You can reasonably argue that the higher-level code is in the wrong, but it’s best if the compressor makes this impossible by satisfying the restriction that all data in the decompressed buffer “originates" in the encoded buffer, i.e. it is either a decoded literal, or a match whose source is necessarily at a valid offset in the decompressed buffer.

Note that even in the fastest implementations of LZ4, this generally only adds ~0.1 cycles / byte or so (around 10% slowdown).  This cost is well worth avoiding security bugs.  For ZSTD, it should be quite a bit cheaper.

– Steve