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

List:       busybox
Subject:    [PATCH] ash: Fix use-after-free on idx variable
From:       soeren () soeren-tempel ! net
Date:       2022-06-01 13:11:54
Message-ID: 20220601131153.30200-1-soeren () soeren-tempel ! net
[Download RAW message or body]

From: Sören Tempel <soeren+git@soeren-tempel.net>

Consider the following code from ash.c:

	STPUTC(*idx, expdest);
	if (quotes && (unsigned char)*idx == CTLESC) {

The idx variable points to a value in the stack string (as managed
by STPUTC). STPUTC may resize this stack string via realloc(3). If
this happens, the idx pointer needs to be updated. Otherwise,
dereferencing idx may result in a use-after free.

The valgrind output for this edge case looks as follows:

	Invalid read of size 1
	   at 0x113AD7: subevalvar (ash.c:7326)
	   by 0x112EC7: evalvar (ash.c:7674)
	   by 0x113219: argstr (ash.c:6891)
	   by 0x113D10: expandarg (ash.c:8098)
	   by 0x118989: evalcommand (ash.c:10377)
	   by 0x116744: evaltree (ash.c:9373)
	   by 0x1170DC: cmdloop (ash.c:13577)
	   by 0x1191E4: ash_main (ash.c:14756)
	   by 0x10CB3B: run_applet_no_and_exit (appletlib.c:967)
	   by 0x10CBCA: run_applet_and_exit (appletlib.c:986)
	   by 0x10CBCA: main (appletlib.c:1126)
	 Address 0x48b4099 is 857 bytes inside a block of size 2,736 free'd
	   at 0x48A6FC9: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
	   by 0x125B03: xrealloc (xfuncs_printf.c:61)
	   by 0x10F9D2: growstackblock (ash.c:1736)
	   by 0x10FA4E: growstackstr (ash.c:1775)
	   by 0x10FA71: _STPUTC (ash.c:1816)
	   by 0x113A94: subevalvar (ash.c:7325)
	   by 0x112EC7: evalvar (ash.c:7674)
	   by 0x113219: argstr (ash.c:6891)
	   by 0x113D10: expandarg (ash.c:8098)
	   by 0x118989: evalcommand (ash.c:10377)
	   by 0x116744: evaltree (ash.c:9373)
	   by 0x1170DC: cmdloop (ash.c:13577)
	 Block was alloc'd at
	   at 0x48A26D5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
	   by 0x125AE9: xmalloc (xfuncs_printf.c:50)
	   by 0x10ED56: stalloc (ash.c:1622)
	   by 0x10F9FF: growstackblock (ash.c:1746)
	   by 0x10FB2A: growstackto (ash.c:1783)
	   by 0x10FB47: makestrspace (ash.c:1795)
	   by 0x10FDE7: memtodest (ash.c:6390)
	   by 0x10FE91: strtodest (ash.c:6417)
	   by 0x112CC5: varvalue (ash.c:7558)
	   by 0x112D80: evalvar (ash.c:7603)
	   by 0x113219: argstr (ash.c:6891)
	   by 0x113D10: expandarg (ash.c:8098)

This patch fixes this issue by updating the pointers again via
the restart label if STPUTC re-sized the stack. This issue
has been reported to us at Alpine Linux downstream.

See also:

* https://gitlab.alpinelinux.org/alpine/aports/-/issues/13900
* http://lists.busybox.net/pipermail/busybox/2022-April/089655.html
---
 shell/ash.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/shell/ash.c b/shell/ash.c
index ef4a47afe..924729a64 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7323,6 +7323,8 @@ subevalvar(char *start, char *str, int strloc,
 				if (idx >= end)
 					break;
 				STPUTC(*idx, expdest);
+				if (stackblock() != restart_detect)
+					goto restart;
 				if (quotes && (unsigned char)*idx == CTLESC) {
 					idx++;
 					len++;
_______________________________________________
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]