Repository navigation
[BUG] Lost error in git_config_list_dup on allocation failure #7417
Description
libgit2 commit 07b89d1 src/libgit2/config_list.c:105.
Bug
The dup loop does not assign the result of git_config_list_dup_entry to error. When an allocation inside it fails, the function frees the partial list and returns 0, which reports success even though *out was never set:
for (head = config_list->entries; head; head = head->next)
if ((git_config_list_dup_entry(result, &head->entry->base.entry)) < 0) /* 105: error stays 0 */
goto out;
...
out:
git_config_list_free(result);
return error; /* success, *out unset */The caller config_file_iterator (config_file.c:278) then passes the NULL dupped list to git_config_list_iterator_new, which dereferences it at config_list.c:255 (it->head = config_list->entries). The process gets SIGSEGV where it should get -1. The fix is error = git_config_list_dup_entry(...).
Reproduce
01.c installs a counting allocator through GIT_OPT_SET_ALLOCATOR that returns NULL for the Nth allocation, which looks exactly like a real OOM to libgit2. It opens a small multi-entry config, then runs git_config_snapshot with the Nth allocation failing, for N = 1..200. Only allocations made during the snapshot can fail, so the scan does not stop first at the unchecked git_config_list_append allocation in config parsing (see 00-config_list_append-nullderef).
gcc -g -O0 01.c -I"$LIBGIT2_INCLUDE" "$LIBGIT2_LIBDIR/libgit2.a" -lz -lpthread -ldl -o 01_c
./01_c # scan
gdb -ex run -ex bt --args ./01_c <cfg> <N> # single index alloc #0: OK
alloc #1: clean error (handled)
...
alloc #6: clean error (handled)
Segmentation fault # N = 7, exit 139
#0 git_config_list_iterator_new src/libgit2/config_list.c:255
#1 config_file_iterator src/libgit2/config_file.c:280
#2 config_snapshot_open src/libgit2/config_snapshot.c:149
#3 git_config_add_backend src/libgit2/config.c:377
#4 git_config_snapshot src/libgit2/config.c:200
01.c
#include <git2.h>
#include <git2/sys/alloc.h>
#include <stdio.h>
#include <stdlib.h>
static long fail_at, count; /* fail the fail_at-th allocation; 0 = never */
static int should_fail(void) { return fail_at && ++count == fail_at; }
static void *x_malloc(size_t n, const char *file, int line)
{
(void)file; (void)line;
return should_fail() ? NULL : malloc(n ? n : 1);
}
static void *x_realloc(void *p, size_t n, const char *file, int line)
{
(void)file; (void)line;
return should_fail() ? NULL : realloc(p, n ? n : 1);
}
/* Returns 0 on success, <0 on a cleanly reported error; the bug crashes. */
static int run(const char *path, long n)
{
git_config *cfg, *snap;
int rc;
if ((rc = git_config_open_ondisk(&cfg, path)) == 0) {
fail_at = n, count = 0;
rc = git_config_snapshot(&snap, cfg);
fail_at = 0;
git_config_free(cfg);
if (rc == 0)
git_config_free(snap);
}
printf(" alloc #%ld: %s\n", n, rc ? "clean error (handled)" : "OK");
return rc;
}
int main(int argc, char **argv)
{
const char *path = argc > 1 ? argv[1] : "/tmp/01_c.cfg";
long only = argc > 2 ? atol(argv[2]) : 0, n;
git_allocator alloc = { x_malloc, x_realloc, free };
FILE *f = fopen(path, "w");
if (!f)
return perror(path), 1;
/* Several entries so the dup loop runs more than once. */
fputs("[core]\n\tfilemode = true\n\tbare = false\n"
"[user]\n\tname = someone\n\temail = someone@example.com\n", f);
fclose(f);
setvbuf(stdout, NULL, _IONBF, 0);
if (git_libgit2_init() < 0 || git_libgit2_opts(GIT_OPT_SET_ALLOCATOR, &alloc) ||
run(path, 0))
return fprintf(stderr, "01_c: setup failed\n"), 1;
if (only)
return run(path, only) ? 2 : 0;
for (n = 1; n <= 200; n++)
run(path, n);
puts("01_c: no crash -- bug did NOT reproduce");
return 1;
}
Activity