[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