[PATCH v3] ash: fix out-of-bounds read in ifsbreakup()

Denys Vlasenko vda.linux at googlemail.com
Tue Jul 7 09:33:50 UTC 2026


Good day Roberto,

On Mon, Jul 6, 2026 at 2:47 PM Roberto A. Foglietta
<roberto.foglietta at gmail.com> wrote:
> On Mon, 6 Jul 2026 at 11:35, Sanghyun Park via busybox <busybox at busybox.net> wrote:
> > Hi Denys,
> >
> > Thanks for applying it.
> >
> > Here's my reproducer for this bug:
> >
> > base64 -d > poc.sh <<'EOF'
> > IyEvYmJiAS0tIGE6YjpjCmVjZSA8PCB2Yf+AYWJsZSBhbmQ7YUVPRgpoZW4KRU9cCmVjaG8AJC0tLS0tLS0tYjpjCiR7XzooKDdiACByZWEtLS1iOmMKJQskJCQIJCQkCCQkI3t9Ch0/Yw==
> > EOF
> >
> > You could reproduce it with ASAN as follows:
> > ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:halt_on_error=1 ./busybox ash ./poc.sh
> >
> > On my pre-patch BusyBox v1.38.0.git build with ASAN, this reports:
> >
> > ERROR: AddressSanitizer: heap-buffer-overflow
> > READ of size 1
> > #0 ifsbreakup
> >
>
> Denys, you forgot to apply the patch that fixes the fix.
>
> https://github.com/robang74/busybox/commit/7f571a1110edf4c4c7c860eb28e9079f49f88572
>
> commit 7f571a1110edf4c4c7c860eb28e9079f49f88572
> Author: Roberto A. Foglietta <roberto.foglietta at gmail.com>
> Date:   Thu Jun 18 12:41:32 2026 +0200
>
>     ash: fix out-of-bounds read in ifsbreakup(), p.4
>
>     Potentially the longjmp() can call a trap running another script
>     branch in the same process and same environment with the same IFS
>     untouched which might lead to the same error/exception that call
>     the handler itself creates an infinite loop, a stack overflow.
>
>     Even if this case couldn't happen today within the current ash code
>     future changes might relax the constraints and open a potentially
>     corner case hard to catch. So, in the best case, it should be a
>     kind of defensive coding by strictly following a rewind logic.
>
>     Accidentally this patch is a bugfix because every !fatal exception
>     requires doing ifsfree() immediately after the branch interruption
>     before continuing executing in another one. Apart exit(), in fact.
>
>     Requires:
>     - ash: fix out-of-bounds read in ifsbreakup() v3
>
>     Signed-off-by: Roberto A. Foglietta <roberto.foglietta at gmail.com>
>
> diff --git a/shell/ash.c b/shell/ash.c
> index 6ee1b3fca..b465a74b1 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -6014,10 +6014,10 @@ static void
>  restore_handler_expandarg(struct jmploc *savehandler, int err)
>  {
>         exception_handler = savehandler;
> -       if (err) {
> +       ifsfree(); // RAF: main reason of this handler existence
> +       if (err) { // always do it before the longjmp() --> trap
>                 if (exception_type != EXERROR)
>                         longjmp(exception_handler->loc, 1);
> -               ifsfree();
>         }
>  }

dash does not have the above fix in their today's git.

Are they discussing it?


More information about the busybox mailing list