From 186cf4976768029113cf8438734a65bf2c489c5c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 27 Jul 2018 12:14:39 +0200 Subject: [PATCH] hush: in some cases, expand_on_ifs() relied of uninitialized memory The n > 0 check to prevent access to the last byte of non-existing argv[-1] wasn't enough. Switched to making sure there are initialized (zero) bytes there. A predictable testcase is rather hard to construct, unfortunately, contents of memory depends on allocator behavior and whatnot. function old new delta o_save_ptr_helper 119 137 +18 expand_on_ifs 345 339 -6 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 18/-6) Total: 12 bytes Signed-off-by: Denys Vlasenko --- shell/hush.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 02fb1b5ef..1ac2db381 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -3069,6 +3069,13 @@ static int o_save_ptr_helper(o_string *o, int n) o->data = xrealloc(o->data, o->maxlen + 1); list = (char**)o->data; memmove(list + n + 0x10, list + n, string_len); + /* + * expand_on_ifs() has a "previous argv[] ends in IFS?" + * check. (grep for -prev-ifs-check-). + * Ensure that argv[-1][last] is not garbage + * but zero bytes, to save index check there. + */ + list[n + 0x10 - 1] = 0; o->length += 0x10 * sizeof(list[0]); } else { debug_printf_list("list[%d]=%d string_start=%d\n", @@ -5797,12 +5804,16 @@ static int expand_on_ifs(o_string *output, int n, const char *str) /* Start new word... but not always! */ /* Case "v=' a'; echo ''$v": we do need to finalize empty word: */ if (output->has_quoted_part - /* Case "v=' a'; echo $v": + /* + * Case "v=' a'; echo $v": * here nothing precedes the space in $v expansion, * therefore we should not finish the word * (IOW: if there *is* word to finalize, only then do it): + * It's okay if this accesses the byte before first argv[]: + * past call to o_save_ptr() cleared it to zero byte + * (grep for -prev-ifs-check-). */ - || (n > 0 && output->data[output->length - 1]) + || output->data[output->length - 1] ) { new_word: o_addchr(output, '\0');