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

[Jeff King <peff@peff.net>]

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.

> 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