Discussion:
[PATCH] ash: ensure variables are fully initialised when unset
Ron Yorston
2018-11-12 12:51:29 UTC
Permalink
When a variable is unset by calling setvar(name, NULL, 0) the code
to initialise the new, empty variable omits the trailing '='.

Attempts to read the contents of the unset variable will result
in the uninitialised character at the end of the string being
accessed.

For example, running BusyBox under Valgrind and unsetting PATH:

$ valgrind ./busybox_unstripped sh
==21249== Memcheck, a memory error detector
==21249== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==21249== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==21249== Command: ./busybox_unstripped sh
==21249==
/data2/git/build_fix_8721 $ unset PATH
/data2/git/build_fix_8721 $ 0
==21249== Conditional jump or move depends on uninitialised value(s)
==21249== at 0x451371: path_advance (ash.c:2555)
==21249== by 0x456E22: find_command (ash.c:13407)
==21249== by 0x458425: evalcommand (ash.c:10139)
==21249== by 0x454CBC: evaltree (ash.c:9131)
==21249== by 0x456C80: cmdloop (ash.c:13164)

Closes https://bugs.busybox.net/show_bug.cgi?id=8721

Signed-off-by: Ron Yorston <***@pobox.com>
---
shell/ash.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index 90eaf6faf..015f36efb 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -2422,10 +2422,9 @@ setvar(const char *name, const char *val, int flags)
INT_OFF;
nameeq = ckmalloc(namelen + vallen + 2);
p = mempcpy(nameeq, name, namelen);
- if (val) {
- *p++ = '=';
+ *p++ = '=';
+ if (val)
p = mempcpy(p, val, vallen);
- }
*p = '\0';
vp = setvareq(nameeq, flags | VNOSAVE);
INT_ON;
--
2.19.1
Loading...