On Sat, 28 Jun 2025 18:05:31 +0300 Nadav Tasher <[email protected]> wrote:
> On Fri, Jun 27, 2025 at 10:02:25PM +0200, tito wrote: > > On Fri, 27 Jun 2025 21:09:07 +0200 > > "Roberto A. Foglietta" <[email protected]> wrote: > > > > > On Fri, 27 Jun 2025 at 18:36, Xabier Oneca -- xOneca <[email protected]> > > > wrote: > > > > > > > > Hi all, > > > > > > > > > Thinking about this more, I think this introduces a massive security > > > > > vulnerability that it starts allowing shell execution for accounts > > > > > that > > > > > specify a shell of /sbin/nologin or equivalent. > > > > > > > > This is a *very important* and scary observation. I think this can be > > > > a blocker for this patch as-is. > > > > > > > > > > Which is more or less the same problem we can have when busybox is > > > suid root and every user can escalate privileges by calling it. > > HI, > > If i recall it correctly when i played with distros on a floppy > > the suid binary was a separate one from the main busybox. > > Sometimes there was even 3 different ones: > > one with all standard commands with links in /bin and /usr/bin > > a separate su binary (busybox or other) that did require a root password > > and a suid binary with links in /sbin. > > > > > Let me guess about suid escalation: this is not a flaw in BusyBox's > > > design, but rather a misconfiguration of the system. > > > > > > Therefore, installing a "standalone busybox" into a common > > > users-shared privilege-aware system, what can be? A massive > > > vulnerability by design or a system misconfiguration? > > > > > > Anyway, a vulnerability does not exist until a PoC is presented. > > > However, in cautelativa way into the menuconfig a warning should be > > > inserted like > > > > > > - this option might lead to user shell escalation (before the PoC) > > > > > > - this option leads to user shell escalation (after the PoC) > > > > > > Those who see in this a problem (people who miscofigure their system > > > for no reason rather than do things they do not know about) are > > > invited to propose a solution within the constraints of the > > > "standalone" concept. Participation is the key. > > > > Couldn't this be solved inside this get_shell_name function? for example: > > (not that I looked at it.... :) ) > > > > 1) query /etc/passwd for user's pw->pw-shell > > 2) if it is /bin/sh use whatever shell is aliased to it in busybox if any > > 3) if it is /bin/bash use whatever shell is aliased to it in busybox if any > > 4) if it is /bin/ash use busybox ash if the applet exists > > 5) if it is /bin/hush use busybox hush if the applet exists > > 6) if it doesn't match any of the previous (e.g. /bin/nologin, /bin/true) > > use the applet with that name if it exists > > 7) in all other cases throw a file not found error (and die?). > > > > I'm sure this is a horrible idea and that I overlooked something very > > important and obvious..... > > So these are just my 0.2 cents of participation. > > > > Ciao, > > Tito > > > > > Best regards, R- > > > _______________________________________________ > > > busybox mailing list > > > [email protected] > > > https://lists.busybox.net/mailman/listinfo/busybox > > > > _______________________________________________ > > busybox mailing list > > [email protected] > > https://lists.busybox.net/mailman/listinfo/busybox > > Hi, replying in bulk to all of you :) > > I tend to agree that skipping get_shell_name() is dangerous because of the > nologin > case. I have completely overlooked that, so thank you for bringing it up. > > I might have an idea that would still allow this patchset to work, but > without this patch: > Looking at the BASH_IS_ASH and BASH_IS_HUSH Kconfig entries, I've come up > with the idea to > allow certin applets to be aliased to ash or hush. Could be nice to have a > Kconfig called > ALERTNALIVE_SHELL_NAMES that allows to set multiple aliases to ash or hush, > thus allowing > a more explicit and secure way to enforce internal-shell execution. > > If you look at my previous patch proposing the bb_exec calls, you'll find the > newly > proposed FEATURE_TRY_BASENAME_APPLETS, you'll learn that it allows bb_exec to > search > for applets by the basename of the path provided (an experimental feature). > This essentially allows get_shell_name() to return whatever shell is desired > by the > system, yet still have bb_exec execute it internally if needed, since it > might exist as > an applet, executable by the basename of the path returned by > get_shell_name(). > > For example, let's say get_shell_name() returns /bin/bash, and BASH_IS_ASH is > enabled, > bb_exec will look for bash, and end up executing ash - the desired behaviour. > If it returns /sbin/nologin, then bb_exec will search for nologin, and if it > is found, > execute it. If not, it'll just fall-back to /sbin/nologin. Shouldn't this be an error if we enforce busybox as self-contained? In other words we don't execute anything outside busybox or am I misunderstanding what self-contained means? > > What I'm proposing, is a middleground where one can define more aliases for > the internal > shells, allowing get_shell_name() to return /bin/dash or /bin/zsh and still > execute ash or > hush. > Dosen't this open a compatibility nightmare of different behaviors between shells, missing builtins, expected bashisms...... > This feature could even replace the BASH_IS_{ASH, HUSH} Kconfig entries if > desired. > > WDYT? > > Appreciate your participation, > Nadav _______________________________________________ busybox mailing list [email protected] https://lists.busybox.net/mailman/listinfo/busybox
