From bd7ca10fdd762f8846dc0ef9db0a723bb8f3610a Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Mon, 17 Oct 2022 21:15:24 -0500 Subject: [PATCH] fix(qflist): avoid read of uninitialized memory (#20709) If the call to `qf_setup_state()` in `qf_init_ext()` fails, control flow jumps to label `qf_init_end` where a call to `qf_update_buffer()` is made with `old_last` as a function call argument. Prior to this patch, `old_last` would not yet have been initialized to its default value of `NULL`, resulting in `qf_update_buffer()` attempting to compare against its uninitialized value (quickfix.c:3882) then later forwarding it to `qf_fill_buffer()` where the address is dereferenced and repeatedly read from/ written to for performing core qflist operations. Depending on what the default value of `old_last` was, the results may range from a best case scenario of neovim terminating with SIGSEGV from an attempt to dereference an invalid pointer (quickfix.c:4056) to memory corruption if it contained some other value that results in `qfp` being initialized from `old_last->qf_next` (after which it is subsequently written to and read from in a fairly arbitrary fashion). Though extremely unlikely, it's possible for there to be security considerations as a user can ensure that the next call to `qf_setup_state()` fails. This patch ensures that `old_last` is NULL-assigned before control flow jumps to `qf_init_end`. Closes #20639. --- src/nvim/quickfix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c index 2d8100595f..af1db1956b 100644 --- a/src/nvim/quickfix.c +++ b/src/nvim/quickfix.c @@ -1059,6 +1059,7 @@ static int qf_init_ext(qf_info_T *qi, int qf_idx, const char *restrict efile, bu { qfstate_T state = { 0 }; qffields_T fields = { 0 }; + qfline_T *old_last = NULL; static efm_T *fmt_first = NULL; static char *last_efm = NULL; int retval = -1; // default: return error flag @@ -1072,7 +1073,6 @@ static int qf_init_ext(qf_info_T *qi, int qf_idx, const char *restrict efile, bu } qf_list_T *qfl; - qfline_T *old_last = NULL; bool adding = false; if (newlist || qf_idx == qi->qf_listcount) { // make place for a new list