[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: usrmerge breaks POSIX



On 2024-02-15 16:30:25 -0800, Russ Allbery wrote:
> I was only able to find this discussion of why pkexec checks $SHELL, and
> it doesn't support my assumption that it was an intentional security
> measure, so I may well be wrong in that part of my analysis.  Apologies
> for that; I clearly should have done more research.  git blame points to a
> commit that only references this thread:
> 
> https://lists.freedesktop.org/archives/polkit-devel/2009-December/000282.html
> 
> which seems to imply that this was done to match sudo behavior and because
> the author believed this was the right way to validate the SHELL setting.

This is the kind of information that should have been put as a comment
in the source code. But the pkexec code seems to be buggy because sudo
does *not* check that the $SHELL value is in /etc/shells?

In the sudo(8) man page:

  -s, --shell
     Run the shell specified by the SHELL environment variable if it
     is set or the shell specified by the invoking  user's  password
     database entry. [...]
[...]
  SHELL            Used to determine shell to run with -s option.

and there is no mention of /etc/shells.

/etc/shells is mentioned in sudoers(5):

  runas_check_shell
    If enabled, sudo will only run  commands  as  a  user
    whose  shell appears in the /etc/shells file, even if
    the invoking user's Runas_List would otherwise permit
    it. [...]

So this concerns the login shell of the target user, not the $SHELL
value. This is confirmed by the sudo source:

/*
 * Returns true if the user's shell is considered to be valid.
 */
bool
user_shell_valid(const struct passwd *pw)
{
    debug_decl(user_shell_valid, SUDOERS_DEBUG_NSS);

    if (!def_runas_check_shell)
        debug_return_bool(true);

    debug_return_bool(valid_shell(pw->pw_shell));
}

where valid_shell() does

    while ((entry = CALL(getusershell)()) != NULL) {
        if (strcmp(entry, shell) == 0)
            debug_return_bool(true);
    }

But chsh will set the login shell to a pathname from /etc/shells,
so that there are no issues with aliases pathnames in this case.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)


Reply to: