untracked-cache: be defensive about missing NULs in index

The on-disk format for the untracked-cache extension contains
NUL-terminated filenames. We parse these from the mmap'd file using
string functions like strlen(). This works fine in the normal case, but
if we see a malformed or corrupted index, we might read off the end of
our mmap.

Instead, let's use memchr() to find the trailing NUL within the bytes we
know are available, and return an error if it's missing.

Note that we can further simplify by folding another range check into
our conditional. After we find the end of the string, we set "next" to
the byte after the string and treat it as an error if there are no such
bytes left. That saves us from having to do a range check at the
beginning of each subsequent string (and works because there is always
data after each string). We can do both range checks together by
checking "!eos" (we didn't find a NUL) and "eos == end" (it was on the
last available byte, meaning there's nothing after). This replaces the
existing "next > end" checks.

Note also that the decode_varint() calls have a similar problem (we
don't even pass them "end"; they just keep parsing). These are probably
OK in practice since varints have a finite length (we stop parsing when
we'd overflow a uintmax_t), so the worst case is that we'd overflow into
reading the trailing bytes of the index.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/dir.c b/dir.c
index f5293a6536e5f21be161c093c0695c252972cdd1..7b0513c4766491d0090adb09ce3764d9c71c666b 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -2733,6 +2733,7 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,
 {
        struct untracked_cache_dir ud, *untracked;
        const unsigned char *next, *data = rd->data, *end = rd->end;
 {
        struct untracked_cache_dir ud, *untracked;
        const unsigned char *next, *data = rd->data, *end = rd->end;

+       const unsigned char *eos;

        unsigned int value;
        int i, len;
 
        unsigned int value;
        int i, len;
 


@@ -2756,21 +2757,24 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,
        ALLOC_ARRAY(ud.dirs, ud.dirs_nr);
        data = next;
 
        ALLOC_ARRAY(ud.dirs, ud.dirs_nr);
        data = next;
 

-       len = strlen((const char *)data);
-       next = data + len + 1;
-       if (next > rd->end)
+       eos = memchr(data, '\0', end - data);
+       if (!eos || eos == end)

                return -1;
                return -1;

+       len = eos - data;
+       next = eos + 1;
+

        *untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), len, 1));
        memcpy(untracked, &ud, sizeof(ud));
        memcpy(untracked->name, data, len + 1);
        data = next;
 
        for (i = 0; i < untracked->untracked_nr; i++) {
        *untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), len, 1));
        memcpy(untracked, &ud, sizeof(ud));
        memcpy(untracked->name, data, len + 1);
        data = next;
 
        for (i = 0; i < untracked->untracked_nr; i++) {

-               len = strlen((const char *)data);
-               next = data + len + 1;
-               if (next > rd->end)
+               eos = memchr(data, '\0', end - data);
+               if (!eos || eos == end)

                        return -1;
                        return -1;

-               untracked->untracked[i] = xstrdup((const char*)data);
+               len = eos - data;
+               next = eos + 1;
+               untracked->untracked[i] = xmemdupz(data, len);

                data = next;
        }
 
                data = next;
        }