From fa52ac9781f479de8ab4d8526276244c0a0471f4 Mon Sep 17 00:00:00 2001 From: Sören Tempel Date: Mon, 28 Feb 2022 08:36:50 +0100 Subject: ash: don't read past end of var in subvareval for bash substitutions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this patch, BusyBox handles bash pattern substitutions without a terminating '/' character incorrectly. Consider the following shell script: _bootstrapver=5.0.211-r0 _referencesdir="/usr/${_bootstrapver/-*}/Sources" echo $_referencesdir This should output `/usr/5.0.211/Sources`. However, without this patch it instead outputs `/usr/5.0.211Sources`. This is due to the fact that BusyBox expects the bash pattern substitutions to always be terminated with a '/' (at least in this part of subvareval) and thus reads passed the substitution itself and consumes the '/' character which is part of the literal string. If there is no '/' after the substitution then BusyBox might perform an out-of-bounds read under certain circumstances. When replacing the bash pattern substitution with `${_bootstrapver/-*/}`, or with this patch applied, ash outputs the correct value. Signed-off-by: Sören Tempel Signed-off-by: Denys Vlasenko --- shell/ash.c | 4 ++++ shell/ash_test/ash-vars/var_bash_repl_unterminated.right | 1 + shell/ash_test/ash-vars/var_bash_repl_unterminated.tests | 2 ++ shell/hush_test/hush-vars/var_bash_repl_unterminated.right | 1 + shell/hush_test/hush-vars/var_bash_repl_unterminated.tests | 2 ++ 5 files changed, 10 insertions(+) create mode 100644 shell/ash_test/ash-vars/var_bash_repl_unterminated.right create mode 100755 shell/ash_test/ash-vars/var_bash_repl_unterminated.tests create mode 100644 shell/hush_test/hush-vars/var_bash_repl_unterminated.right create mode 100755 shell/hush_test/hush-vars/var_bash_repl_unterminated.tests (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index adb0f223a..54335c5dd 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7081,6 +7081,10 @@ subevalvar(char *start, char *str, int strloc, *repl = '\0'; break; } + if ((unsigned char)*repl == CTLENDVAR) { /* ${v/pattern} (no trailing /, no repl) */ + repl = NULL; + break; + } /* Handle escaped slashes, e.g. "${v/\//_}" (they are CTLESC'ed by this point) */ if ((unsigned char)*repl == CTLESC && repl[1]) repl++; diff --git a/shell/ash_test/ash-vars/var_bash_repl_unterminated.right b/shell/ash_test/ash-vars/var_bash_repl_unterminated.right new file mode 100644 index 000000000..5bff3a6fa --- /dev/null +++ b/shell/ash_test/ash-vars/var_bash_repl_unterminated.right @@ -0,0 +1 @@ +b/d diff --git a/shell/ash_test/ash-vars/var_bash_repl_unterminated.tests b/shell/ash_test/ash-vars/var_bash_repl_unterminated.tests new file mode 100755 index 000000000..c9513343d --- /dev/null +++ b/shell/ash_test/ash-vars/var_bash_repl_unterminated.tests @@ -0,0 +1,2 @@ +a=b-c +echo ${a/-*}/d diff --git a/shell/hush_test/hush-vars/var_bash_repl_unterminated.right b/shell/hush_test/hush-vars/var_bash_repl_unterminated.right new file mode 100644 index 000000000..5bff3a6fa --- /dev/null +++ b/shell/hush_test/hush-vars/var_bash_repl_unterminated.right @@ -0,0 +1 @@ +b/d diff --git a/shell/hush_test/hush-vars/var_bash_repl_unterminated.tests b/shell/hush_test/hush-vars/var_bash_repl_unterminated.tests new file mode 100755 index 000000000..c9513343d --- /dev/null +++ b/shell/hush_test/hush-vars/var_bash_repl_unterminated.tests @@ -0,0 +1,2 @@ +a=b-c +echo ${a/-*}/d -- cgit v1.2.3-55-g6feb From 7750b5a25a8cf9081b7c248687c876d0068e85bb Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 1 Mar 2022 09:56:54 +0100 Subject: ash: fix unsafe use of mempcpy function old new delta subevalvar 1549 1557 +8 Signed-off-by: Denys Vlasenko --- shell/ash.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 54335c5dd..44ec2eafd 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7191,7 +7191,13 @@ subevalvar(char *start, char *str, int strloc, len = orig_len - pos; if (!quotes) { - loc = mempcpy(startp, startp + pos, len); + /* want: loc = mempcpy(startp, startp + pos, len) + * but it does not allow overlapping arguments */ + loc = startp; + while (--len >= 0) { + *loc = loc[pos]; + loc++; + } } else { for (vstr = startp; pos != 0; pos--) { if ((unsigned char)*vstr == CTLESC) -- cgit v1.2.3-55-g6feb From 5fe20cf3212fbada86fb75cf13064caed6a5f3a9 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 1 Mar 2022 10:08:59 +0100 Subject: ash: do not truncate failed tilde expansion on unknown user names Do not skip over "*p = c;" statement. Testcase: echo ~~nouser/qwe function old new delta argstr 1396 1406 +10 Signed-off-by: Denys Vlasenko --- shell/ash.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 44ec2eafd..ef4a47afe 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -6532,9 +6532,7 @@ exptilde(char *startp, int flag) home = lookupvar("HOME"); } else { pw = getpwnam(name); - if (pw == NULL) - goto lose; - home = pw->pw_dir; + home = pw ? pw->pw_dir : NULL; } *p = c; if (!home) -- cgit v1.2.3-55-g6feb