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

From: Jeff King <peff@peff.net>
Date: Jun 15, 2018, 03:28 UTC
Message-ID: <20180615032850.GA23241@sigill.intra.peff.net>
In-Reply-To: <2067D731-C415-4D19-8CDA-90D7DC638397@l4w.io>

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