[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] ash: don't read past end of var in subvareval for bash substitutions
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2022-03-01 7:48:19
Message-ID: CAK1hOcPWZ+da4iUQeg6_OdQuGcFFvDsSu1QMD8fABHfKs00xiw () mail ! gmail ! com
[Download RAW message or body]

On Mon, Feb 28, 2022 at 8:38 AM <soeren@soeren-tempel.net> wrote:
> From: Sören Tempel <soeren@soeren-tempel.net>
>
> 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 <soeren@soeren-tempel.net>
> ---
>  shell/ash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index adb0f223a..8097d51c3 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7077,7 +7077,7 @@ subevalvar(char *start, char *str, int strloc,
>                                 repl = NULL;
>                                 break;
>                         }
> -                       if (*repl == '/') {
> +                       if (*repl == '/' || (unsigned char)*repl == CTLENDVAR) {
>                                 *repl = '\0';
>                                 break;
>                         }

This broke shell/ash_test/ash-vars/var_bash5.tests

I'm going to use this:

                        if (*repl == '/') {
                                *repl = '\0';
                                break;
                        }
+                       if ((unsigned char)*repl == CTLENDVAR) { /*
${v/pattern} (no trailing /, no repl) */
+                               repl = NULL;
+                               break;
+                       }
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread]