commit bd7ca10fdd762f8846dc0ef9db0a723bb8f3610a
parent be43cf16cc677cf50f31cbbf75bc028ce36b8921
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Mon, 17 Oct 2022 21:15:24 -0500
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.
Diffstat:
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git 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