Skip to content

Commit c6909f9

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3a7b45a commit c6909f9

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

dir.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2733,6 +2733,7 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,
27332733
{
27342734
struct untracked_cache_dir ud, *untracked;
27352735
const unsigned char *next, *data = rd->data, *end = rd->end;
2736+
const unsigned char *eos;
27362737
unsigned int value;
27372738
int i, len;
27382739

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

2759-
len = strlen((const char *)data);
2760-
next = data + len + 1;
2761-
if (next > rd->end)
2760+
eos = memchr(data, '\0', end - data);
2761+
if (!eos || eos == end)
27622762
return -1;
2763+
len = eos - data;
2764+
next = eos + 1;
2765+
27632766
*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), len, 1));
27642767
memcpy(untracked, &ud, sizeof(ud));
27652768
memcpy(untracked->name, data, len + 1);
27662769
data = next;
27672770

27682771
for (i = 0; i < untracked->untracked_nr; i++) {
2769-
len = strlen((const char *)data);
2770-
next = data + len + 1;
2771-
if (next > rd->end)
2772+
eos = memchr(data, '\0', end - data);
2773+
if (!eos || eos == end)
27722774
return -1;
2773-
untracked->untracked[i] = xstrdup((const char*)data);
2775+
len = eos - data;
2776+
next = eos + 1;
2777+
untracked->untracked[i] = xmemdupz(data, len);
27742778
data = next;
27752779
}
27762780

0 commit comments

Comments
 (0)