Re: security: potential out-of-bound read at ewah_io.c |ewah_read_mmap|

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Fri, Jun 15, 2018 at 06:59:43AM +0800, Luat Nguyen wrote:
>
>> Recently, I’ve found a security issue related to out-of-bound read at function named `ewah_read_mmap`
>
> Thanks, this is definitely a bug worth addressing. But note...
>
>> Assume that, an attacker can put malicious `./git/index` into a repo by somehow.
>
> We generally don't consider .git/index (or pack .bitmap files, which
> also use this implementation) to be a major part of Git's attack
> surface, since they are generated locally. And if you can write to
> somebody's .git directory, there are already much easier ways to execute
> arbitrary code.

Thanks for giving a fair assessment on the gravity of the issue, to
which I agree fully, and also fixes and clean-ups.



>
>> Since there is lack of check whether the remaining size of `ptr`is
>> equal to `buffer_size` or not.
>
> Yep. We also fail to check if we even have enough bytes to read the
> buffer_size in the first place.
>
> Here are some patches. The first one fixes the problem you found. The
> second one drops some dead code that has a related problem. And the
> third just drops some dead code that I noticed in the same file. :)
>
>   [1/3]: ewah_read_mmap: bounds-check mmap reads
>   [2/3]: ewah: drop ewah_deserialize function
>   [3/3]: ewah: drop ewah_serialize_native function
>
>  ewah/ewah_io.c          | 106 ++++++++--------------------------------
>  ewah/ewok.h             |   4 +-
>  t/t5310-pack-bitmaps.sh |  13 +++++
>  3 files changed, 35 insertions(+), 88 deletions(-)
>
> -Peff