emo/busybox
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ash: ensure variables are fully initialised when unset

Browse Source 
When a variable is unset by calling setvar(name, NULL, 0) the code
to initialise the new, empty variable fails to initialise the last
character of the string.

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

v2: On the dash mailing list Harald van Dijk was kind enough to point
    out a flaw in my reasoning and provide an alternative patch.  Sadly
    his patch adds 2 bytes of bloat.  Using xzalloc to zero the whole
    string gives a bloat of -3 bytes.

function                                             old     new   delta
setvar                                               172     169      -3

Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Ron Yorston 2018-11-12 21:10:54 +00:00 committed by Denys Vlasenko
parent 060f0a050a
commit e6a63bf683
1 changed files with 2 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
shell/ash.c
View File

@ -2420,13 +2420,12 @@ setvar(const char *name, const char *val, int flags)
} }
INT_OFF; INT_OFF;
nameeq = ckmalloc(namelen + vallen + 2); nameeq = ckzalloc(namelen + vallen + 2);
p = mempcpy(nameeq, name, namelen); p = mempcpy(nameeq, name, namelen);
if (val) { if (val) {
*p++ = '='; *p++ = '=';
p = mempcpy(p, val, vallen); memcpy(p, val, vallen);
} }
*p = '\0';
vp = setvareq(nameeq, flags | VNOSAVE); vp = setvareq(nameeq, flags | VNOSAVE);
INT_ON; INT_ON;