Skip to content

Commit 58a6fe9

Browse files
committed
index: convert read_entry to return entry size via an out-param
The function `read_entry` does not conform to our usual coding style of returning stuff via the out parameter and to use the return value for reporting errors. Due to most of our code conforming to that pattern, it has become quite natural for us to actually return `-1` in case there is any error, which has also slipped in with commit 5625d86 (index: support index v4, 2016-05-17). As the function returns an `size_t` only, though, the return value is wrapped around, causing the caller of `read_tree` to continue with an invalid index entry. Ultimately, this can lead to a double-free. Improve code and fix the bug by converting the function to return the index entry size via an out parameter and only using the return value to indicate errors. Reported-by: Krishna Ram Prakash R <krp@gtux.in> Reported-by: Vivek Parikh <viv0411.parikh@gmail.com>
1 parent d11c4a1 commit 58a6fe9

File tree

1 file changed

+13
-9
lines changed

1 file changed

+13
-9
lines changed

src/index.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2299,8 +2299,9 @@ static size_t index_entry_size(size_t path_len, size_t varint_len, uint32_t flag
22992299
}
23002300
}
23012301

2302-
static size_t read_entry(
2302+
static int read_entry(
23032303
git_index_entry **out,
2304+
size_t *out_size,
23042305
git_index *index,
23052306
const void *buffer,
23062307
size_t buffer_size,
@@ -2314,7 +2315,7 @@ static size_t read_entry(
23142315
char *tmp_path = NULL;
23152316

23162317
if (INDEX_FOOTER_SIZE + minimal_entry_size > buffer_size)
2317-
return 0;
2318+
return -1;
23182319

23192320
/* buffer is not guaranteed to be aligned */
23202321
memcpy(&source, buffer, sizeof(struct entry_short));
@@ -2356,7 +2357,7 @@ static size_t read_entry(
23562357

23572358
path_end = memchr(path_ptr, '\0', buffer_size);
23582359
if (path_end == NULL)
2359-
return 0;
2360+
return -1;
23602361

23612362
path_length = path_end - path_ptr;
23622363
}
@@ -2386,16 +2387,20 @@ static size_t read_entry(
23862387
entry.path = tmp_path;
23872388
}
23882389

2390+
if (entry_size == 0)
2391+
return -1;
2392+
23892393
if (INDEX_FOOTER_SIZE + entry_size > buffer_size)
2390-
return 0;
2394+
return -1;
23912395

23922396
if (index_entry_dup(out, index, &entry) < 0) {
23932397
git__free(tmp_path);
2394-
return 0;
2398+
return -1;
23952399
}
23962400

23972401
git__free(tmp_path);
2398-
return entry_size;
2402+
*out_size = entry_size;
2403+
return 0;
23992404
}
24002405

24012406
static int read_header(struct index_header *dest, const void *buffer)
@@ -2499,10 +2504,9 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
24992504
/* Parse all the entries */
25002505
for (i = 0; i < header.entry_count && buffer_size > INDEX_FOOTER_SIZE; ++i) {
25012506
git_index_entry *entry = NULL;
2502-
size_t entry_size = read_entry(&entry, index, buffer, buffer_size, last);
2507+
size_t entry_size;
25032508

2504-
/* 0 bytes read means an object corruption */
2505-
if (entry_size == 0) {
2509+
if ((error = read_entry(&entry, &entry_size, index, buffer, buffer_size, last)) < 0) {
25062510
error = index_error_invalid("invalid entry");
25072511
goto done;
25082512
}

0 commit comments

Comments
 (0)