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.
***
daemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin
***
const char* FAST_FUNC get_shell_name(void)
{
#if ENABLE_FEATURE_PREFER_APPLETS && !ENABLE_SH_IS_NONE
/* when "sh" is defined as an applet and applets
* are prefered, always use "sh" to spawn shells */
return "sh";
#else
struct passwd *pw;
char *shell;
shell = getenv("SHELL");
if (shell && shell[0])
return shell;
pw = getpwuid(getuid());
if (pw && pw->pw_shell && pw->pw_shell[0])
return pw->pw_shell;
return DEFAULT_SHELL;
#endif
}
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
