ewah_read_mmap: bounds-check mmap reads

The on-disk ewah format tells us how big the ewah data is,
and we blindly read that much from the buffer without
considering whether the mmap'd data is long enough, which
can lead to out-of-bound reads.

Let's make sure we have data available before reading it,
both for the ewah header/footer as well as for the bit data
itself. In particular:

- keep our ptr/len pair in sync as we move through the
buffer, and check it before each read

- check the size for integer overflow (this should be
impossible on 64-bit, as the size is given as a 32-bit
count of 8-byte words, but is possible on a 32-bit
system)

- return the number of bytes read as an ssize_t instead of
an int, again to prevent integer overflow

- compute the return value using a pointer difference;
this should yield the same result as the existing code,
but makes it more obvious that we got our computations
right

The included test is far from comprehensive, as it just
picks a static point at which to truncate the generated
bitmap. But in practice this will hit in the middle of an
ewah and make sure we're at least exercising this code.

Reported-by: Luat Nguyen <root@l4w.io>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
index bed1994551cd03532fe56913b0e27343834c805f..33c08c40f896f32d9a1c251c1456755e949d91bd 100644 (file)
--- a/ewah/ewah_io.c
+++ b/ewah/ewah_io.c
@@ -122,16 +122,23 @@ int ewah_serialize_strbuf(struct ewah_bitmap *self, struct strbuf *sb)
        return ewah_serialize_to(self, write_strbuf, sb);
 }
 
        return ewah_serialize_to(self, write_strbuf, sb);
 }
 

-int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)
+ssize_t ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)

 {
        const uint8_t *ptr = map;
 {
        const uint8_t *ptr = map;

+       size_t data_len;

        size_t i;
 
        size_t i;
 

+       if (len < sizeof(uint32_t))
+               return error("corrupt ewah bitmap: eof before bit size");

        self->bit_size = get_be32(ptr);
        ptr += sizeof(uint32_t);
        self->bit_size = get_be32(ptr);
        ptr += sizeof(uint32_t);

+       len -= sizeof(uint32_t);

 
 

+       if (len < sizeof(uint32_t))
+               return error("corrupt ewah bitmap: eof before length");

        self->buffer_size = self->alloc_size = get_be32(ptr);
        ptr += sizeof(uint32_t);
        self->buffer_size = self->alloc_size = get_be32(ptr);
        ptr += sizeof(uint32_t);

+       len -= sizeof(uint32_t);

 
        REALLOC_ARRAY(self->buffer, self->alloc_size);
 
 
        REALLOC_ARRAY(self->buffer, self->alloc_size);
 


@@ -141,15 +148,25 @@ int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len)
         * the endianness conversion in a separate pass to ensure
         * we're loading 8-byte aligned words.
         */
         * the endianness conversion in a separate pass to ensure
         * we're loading 8-byte aligned words.
         */

-       memcpy(self->buffer, ptr, self->buffer_size * sizeof(eword_t));
-       ptr += self->buffer_size * sizeof(eword_t);
+       data_len = st_mult(self->buffer_size, sizeof(eword_t));
+       if (len < data_len)
+               return error("corrupt ewah bitmap: eof in data "
+                            "(%"PRIuMAX" bytes short)",
+                            (uintmax_t)(data_len - len));
+       memcpy(self->buffer, ptr, data_len);
+       ptr += data_len;
+       len -= data_len;

 
        for (i = 0; i < self->buffer_size; ++i)
                self->buffer[i] = ntohll(self->buffer[i]);
 
 
        for (i = 0; i < self->buffer_size; ++i)
                self->buffer[i] = ntohll(self->buffer[i]);
 

+       if (len < sizeof(uint32_t))
+               return error("corrupt ewah bitmap: eof before rlw");

        self->rlw = self->buffer + get_be32(ptr);
        self->rlw = self->buffer + get_be32(ptr);

+       ptr += sizeof(uint32_t);
+       len -= sizeof(uint32_t);

 
 

-       return (3 * 4) + (self->buffer_size * 8);
+       return ptr - (const uint8_t *)map;

 }
 
 int ewah_deserialize(struct ewah_bitmap *self, int fd)
 }
 
 int ewah_deserialize(struct ewah_bitmap *self, int fd)

diff --git a/ewah/ewok.h b/ewah/ewok.h
index dc43d05b644af07a86bb64dbc6790ea0897d2f43..357fd93c84edbe79f9bcb10e2947ed935ac9f0df 100644 (file)
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -91,7 +91,7 @@ int ewah_serialize_native(struct ewah_bitmap *self, int fd);
 int ewah_serialize_strbuf(struct ewah_bitmap *self, struct strbuf *);
 
 int ewah_deserialize(struct ewah_bitmap *self, int fd);
 int ewah_serialize_strbuf(struct ewah_bitmap *self, struct strbuf *);
 
 int ewah_deserialize(struct ewah_bitmap *self, int fd);

-int ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len);
+ssize_t ewah_read_mmap(struct ewah_bitmap *self, const void *map, size_t len);

 
 uint32_t ewah_checksum(struct ewah_bitmap *self);
 
 
 uint32_t ewah_checksum(struct ewah_bitmap *self);
 

diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index 20e2473a03b645d690b25598bc0cb2e421034b6c..b11bc392a840e91378b67c0e272c7c05711c9993 100755 (executable)
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -331,4 +331,17 @@ test_expect_success 'pack reuse respects --incremental' '
        git show-index <empty.idx >actual &&
        test_cmp expect actual
 '
        git show-index <empty.idx >actual &&
        test_cmp expect actual
 '

+
+test_expect_success 'truncated bitmap fails gracefully' '
+       git repack -ad &&
+       git rev-list --use-bitmap-index --count --all >expect &&
+       bitmap=$(ls .git/objects/pack/*.bitmap) &&
+       test_when_finished "rm -f $bitmap" &&
+       head -c 512 <$bitmap >$bitmap.tmp &&
+       mv -f $bitmap.tmp $bitmap &&
+       git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
+       test_cmp expect actual &&
+       test_i18ngrep corrupt stderr
+'
+

 test_done
 test_done