From 52eb10c498d93617bea6e703875b76d6789cc78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20Tempel?= <soeren+git@soeren-tempel.net> Date: Wed, 1 Jun 2022 16:31:53 +0200 Subject: [PATCH] main/busybox: fix yet another use-after-free in BusyBox ash Fixes #13900 --- ...h-Fix-use-after-free-on-idx-variable.patch | 94 +++++++++++++++++++ main/busybox/APKBUILD | 4 +- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 main/busybox/0017-ash-Fix-use-after-free-on-idx-variable.patch diff --git a/main/busybox/0017-ash-Fix-use-after-free-on-idx-variable.patch b/main/busybox/0017-ash-Fix-use-after-free-on-idx-variable.patch new file mode 100644 index 000000000000..22a2578e3964 --- /dev/null +++ b/main/busybox/0017-ash-Fix-use-after-free-on-idx-variable.patch @@ -0,0 +1,94 @@ +From 3813e89e3622b034b0e51acae496493a717555cc Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?S=C3=B6ren=20Tempel?= <soeren+git@soeren-tempel.net> +Date: Wed, 1 Jun 2022 11:51:40 +0200 +Subject: [PATCH] ash: Fix use-after-free on idx variable + +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. + +Also: Move the second realloc-check inside the if statement +that follows so it isn't done twice if the condition evaluates +to false. + +See also: + +* https://gitlab.alpinelinux.org/alpine/aports/-/issues/13900 +* http://lists.busybox.net/pipermail/busybox/2022-April/089655.html +--- + shell/ash.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/shell/ash.c b/shell/ash.c +index ef4a47afe..cbc50eefe 100644 +--- a/shell/ash.c ++++ b/shell/ash.c +@@ -7323,13 +7323,15 @@ 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++; + STPUTC(*idx, expdest); ++ if (stackblock() != restart_detect) ++ goto restart; + } +- if (stackblock() != restart_detect) +- goto restart; + idx++; + len++; + rmesc++; diff --git a/main/busybox/APKBUILD b/main/busybox/APKBUILD index 2bc69230ad21..950a522a04a9 100644 --- a/main/busybox/APKBUILD +++ b/main/busybox/APKBUILD @@ -5,7 +5,7 @@ # Maintainer: Sören Tempel <soeren+alpine@soeren-tempel.net> pkgname=busybox pkgver=1.35.0 -pkgrel=13 +pkgrel=14 pkgdesc="Size optimized toolbox of many common UNIX utilities" url="https://busybox.net/" arch="all" @@ -43,6 +43,7 @@ source="https://busybox.net/downloads/busybox-$pkgver.tar.bz2 0014-ash-fix-use-after-free-in-bash-pattern-substitution.patch 0015-ed-don-t-use-memcpy-with-overlapping-memory-regions.patch 0016-ash-don-t-read-past-end-of-var-in-subvareval-for-bas.patch + 0017-ash-Fix-use-after-free-on-idx-variable.patch 0001-ash-add-built-in-BB_ASH_VERSION-variable.patch @@ -294,6 +295,7 @@ ecbe5c890d966f09280c7eb534109f785c68e292765f17ed7ff62fcc61d20f61443c4155add0a1eb 3eb7609054fa8e03d7e366f7debc5cb0630ff65d521a91be84803bdef3854f81e29d26a9567c501a121e94a55d3a3477894e774508f80def775f2ecc812805e7 0014-ash-fix-use-after-free-in-bash-pattern-substitution.patch 0040800382a6e3adcc6a8094b821488c7e297fc80304afba23a4fca43b7b26ac699378dfbd930ebbf9985336b3e431301f7ca93e2d041a071902a48740d263ef 0015-ed-don-t-use-memcpy-with-overlapping-memory-regions.patch 4c95dc4bf6aff9018bfb52b400f6d8375a1d22493b44ea516cb12dba6556f12797a3cba55768d2e59ff57c0f3247ec1ff95edb8f17561f3d37ec18d83ca47eb0 0016-ash-don-t-read-past-end-of-var-in-subvareval-for-bas.patch +ccdf098fb15eaa316708181469a1193d6eec7067131e7b7645e0219bf03cfd07f4f79e8f62c1e560f6146dcc38186a29bdee08aaa39f290e11d020b8f07d2f65 0017-ash-Fix-use-after-free-on-idx-variable.patch 6d100fe44da2b97c2cbdda253d0504b487212d195144d9315cddbe8c51d18fae3745701923b170b40e35f54b592f94f02cadbffd9cb716661c12a7f1da022763 0001-ash-add-built-in-BB_ASH_VERSION-variable.patch e33dbc27d77c4636f4852d5d5216ef60a9a4343484e4559e391c13c813bf65c782b889914eff2e1f038d74cf02cb0d23824ebbb1044b5f8c86260d5a1bbc4e4d 0001-pgrep-add-support-for-matching-against-UID-and-RUID.patch 2640698e5108434991a8491fcc508bd991d2111b14bb6957385393a36603e1d81fdf826ad7b150d487d2a924630ee54c0fc4f979214e90feca9ba7d2fd96a865 0001-avoid-redefined-warnings-when-building-with-utmps.patch -- GitLab