[git commit] ash: fix out-of-bounds read in ifsbreakup()

[Denys Vlasenko]

commit: https://git.busybox.net/busybox/commit/?id=a448b6d5b21e5b21249391389b6f0551d9bea136
branch: https://git.busybox.net/busybox/log/?h=master

ifsfree() does not only release allocated ifsregion nodes; it also clears
the global IFS region state used by ifsbreakup(). If argstr() raises an
error while expanding an argument, ash longjmps out of expandarg() before
that cleanup runs, leaving stale IFS split offsets behind.

A later expansion can reuse the stack for a shorter string. ifsbreakup()
then sees the stale IFS state, trusts the old offsets, and can walk past
the current stack block before dereferencing p.

Follow dash's root-cause fix: when an expansion-related handler catches
EXERROR and continues, restore the handler and call ifsfree(). Apply
the cleanup to redirectsafe(), expandstr(), and evaltree().

Upstream commit:

    Date:   Mon Dec 5 23:02:01 2022 +0800
    expand: Add ifsfree to expand to fix a logic error that causes a buffer over-read

    On Mon, Jun 20, 2022 at 02:27:10PM -0400, Alex Gorinson wrote:
    > Due to a logic error in the ifsbreakup function in expand.c if a
    > heredoc and normal command is run one after the other by means of a
    > semi-colon, when the second command drops into ifsbreakup the command
    > will be evaluated with the ifslastp/ifsfirst struct that was set when
    > the here doc was evaluated. This results in a buffer over-read that
    > can leak the program's heap, stack, and arena addresses which can be
    > used to beat ASLR.
    >
    > Steps to Reproduce:
    > First bug:
    > cmd args: ~/exampleDir/example> dash
    > $ M='AAAAAAAAAAAAAAAAA'    <note: 17 A's>
    > $ q00(){
    > $ <<000;echo
    > $ ${D?$M$M$M$M$M$M}        <note: 6 $M's>
    > $ 000
    > $ }
    > $ q00                      <note: After the q00 is typed in, the leak
    > should be echo'd out; this works with ash, busybox ash, and dash and
    > with all option args.>
    >
    > Patch:
    > Adding the following to expand.c will fix both bugs in one go.
    > (Thank you to Harald van Dijk and Michael Greenberg for doing the
    > heavy lifting for this patch!)
    > ==========================
    > --- a/src/expand.c
    > +++ b/src/expand.c
    > @@ -859,6 +859,7 @@
    > if (discard)
    > return -1;
    >
    > +ifsfree();
    > sh_error("Bad substitution");
    > }
    >
    > @@ -1739,6 +1740,7 @@
    > } else
    > msg = umsg;
    > }
    > +ifsfree();
    > sh_error("%.*s: %s%s", end - var - 1, var, msg, tail);
    >  }
    > ==========================

    Thanks for the report!

    I think it's better to add the ifsfree() call to the exception
    handling path as other sh_error calls may trigger this too.

function                                             old     new   delta
restore_handler_expandarg                              -      33     +33
evaltree                                             725     711     -14
static.redirectsafe                                  141     124     -17
expandstr                                            262     242     -20
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/3 up/down: 36/-45)            Total: -18 bytes

Signed-off-by: Sanghyun Park <sanghyun.park.cnu at gmail.com>
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 shell/ash.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index fb887f31b..b8ff67b16 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -5574,6 +5574,7 @@ write2pipe(int pip[2], const char *p, size_t len)
 
 /* openhere needs this forward reference */
 static void expandhere(union node *arg);
+static void ifsfree(void);
 static int
 openhere(union node *redir)
 {
@@ -5998,6 +5999,17 @@ redirect(union node *redir, int flags)
 	//	preverrout_fd = copied_fd2;
 }
 
+static void
+restore_handler_expandarg(struct jmploc *savehandler, int err)
+{
+	exception_handler = savehandler;
+	if (err) {
+		if (exception_type != EXERROR)
+			longjmp(exception_handler->loc, 1);
+		ifsfree();
+	}
+}
+
 static int
 redirectsafe(union node *redir, int flags)
 {
@@ -6013,9 +6025,7 @@ redirectsafe(union node *redir, int flags)
 		exception_handler = &jmploc;
 		redirect(redir, flags);
 	}
-	exception_handler = savehandler;
-	if (err && exception_type != EXERROR)
-		longjmp(exception_handler->loc, 1);
+	restore_handler_expandarg(savehandler, err);
 	RESTORE_INT(saveint);
 	return err;
 }
@@ -9792,9 +9802,7 @@ evaltree(union node *n, int flags)
 			trap_depth--;
 			in_trap_ERR = 0;
 
-			exception_handler = savehandler;
-			if (err && exception_type != EXERROR)
-				longjmp(exception_handler->loc, 1);
+			restore_handler_expandarg(savehandler, err);
 
 			exitstatus = savestatus;
 		}
@@ -14009,9 +14017,7 @@ expandstr(const char *ps, int syntax_type)
 	result = stackblock();
 
 out:
-	exception_handler = savehandler;
-	if (err && exception_type != EXERROR)
-		longjmp(exception_handler->loc, 1);
+	restore_handler_expandarg(savehandler, err);
 
 	doprompt = saveprompt;
 	/* Try: PS1='`xxx(`' */