On Sat, Jun 28, 2025 at 06:01:36PM +0200, Roberto A. Foglietta wrote: > On Sat, 28 Jun 2025 at 17:53, Roberto A. Foglietta > <[email protected]> wrote: > > > > On Sat, 28 Jun 2025 at 17:05, Nadav Tasher <[email protected]> wrote: > > > > > 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. > > > > > > > This reported below is the function (with Nadav changes) ignoring > > them, get_shell_name() uses the SHELL environment variable to report > > the shell. Why? Because, in any other case rather than spawning a > > console to have access to a shell, the shell is used to execute > > scripts or commands. So imagine the user daemon that cannot login, but > > can use a shell to execute commands (or crond, which can execute > > scripts). In fact, login does not use (or at least not directly but > > PAM authentication support lets me think that it does not use it > > anyway). Where is the only point in which get_shell_name() can > > interfere with a future login process? Adding a user. > > loginutils/adduser.c:204, at that point which is the shell value > > retrieved and used for the new user? The current user default shell, > > aka the root default shell. In fact, that value is override A) with > > /bin/false when by options the new user has nologin and, b) override > > by the shell passed to adduser command, if any. > > > > Conclusion: get_shell_name() has nothing to do with the login and > > cannot interfere with it. Moreover, if busybox is started at boot time > > and it is part of the booting process, and hence it is the process pid > > zero, and all the other instances are just an applet fork of it > > because it is working as standalone self-contained, then > > get_shell_name() always report the SHELL value, if set (usually not). > > The Nadav patch, is nothing else than setting "sh" as default value of > > "SHELL" variable at the time init starts. Because everyone that uses > > the standalone self-contained mode, likely they have no other binary > > (or system binary) on the initrd image (or whatever they use for that > > role). > > > > This idea that get_shell_name() can interfere with login, needs a > > solid proof of concept to be taken in consideration. Otherwise, should > > be ignored in favour of a more reasonable question: what if the SHELL > > environment variable would always be set with "sh"? This would break > > the system? This would create a massive security violation in the > > mainstream busybox? If yes, then the original busybox has a sort of > > problem, or SHELL usage, in this supposedly massive security breach. > > Instead, if not then also Nadav patch. Prove me wrong, please. > > > > LONG STORY SHORT > > Check the code in libbb/get_shell_name.c > > The Nadav patch about get_shell_name() is equivalent to having the > SHELL environment variable always set as "sh". > > Who think that this would be a massive security breach, then they > should explain why it would be not a problem in the mainstream > busybox, in first place > > Best regards, R-
Roberto, I think what they're trying to say is that in the valid edge-case where you do configure a system to disallow a user login via /etc/passwd, they should not be able to invoke shell executions in any way, which could be bypassed by the proposed change "hardwiring" the shell to ash. The fallback shell is there for when passwd can't be read, which is an entirely different edge-case with different implications. Nadav _______________________________________________ busybox mailing list [email protected] https://lists.busybox.net/mailman/listinfo/busybox
