[PATCH 3/5] exec: Remove recursion from search_binary_handler

[Eric W. Biederman]

Casey Schaufler <casey at schaufler-ca.com> writes:

> On 5/14/2020 7:56 AM, Eric W. Biederman wrote:
>> Kees Cook <keescook at chromium.org> writes:
>>
>>> On Tue, May 12, 2020 at 04:47:14PM -0700, Kees Cook wrote:
>>>> And now I wonder if qemu actually uses the resulting AT_EXECFD ...
>>> It does, though I'm not sure if this is to support crossing mount points,
>>> dropping privileges, or something else, since it does fall back to just
>>> trying to open the file.
>>>
>>>     execfd = qemu_getauxval(AT_EXECFD);
>>>     if (execfd == 0) {
>>>         execfd = open(filename, O_RDONLY);
>>>         if (execfd < 0) {
>>>             printf("Error while loading %s: %s\n", filename, strerror(errno));
>>>             _exit(EXIT_FAILURE);
>>>         }
>>>     }
>> My hunch is that the fallback exists from a time when the kernel did not
>> implement AT_EXECFD, or so that qemu can run on kernels that don't
>> implement AT_EXECFD.  It doesn't really matter unless the executable is
>> suid, or otherwise changes privileges.
>>
>>
>> I looked into this a bit to remind myself why exec works the way it
>> works, with changing privileges.
>>
>> The classic attack is pointing a symlink at a #! script that is suid or
>> otherwise changes privileges.  The kernel will open the script and set
>> the privileges, read the interpreter from the first line, and proceed to
>> exec the interpreter.  The interpreter will then open the script using
>> the pathname supplied by the kernel.  The name of the symlink.
>> Before the interpreter reopens the script the attack would replace
>> the symlink with a script that does something else, but gets to run
>> with the privileges of the script.
>>
>>
>> Defending against that time of check vs time of use attack is why
>> bprm_fill_uid, and cap_bprm_set_creds use the credentials derived from
>> the interpreter instead of the credentials derived from the script.
>>
>>
>> The other defense is to replace the pathname of the executable that the
>> intepreter will open with /dev/fd/N.
>>
>> All of this predates Linux entirely.  I do remember this was fixed at
>> some point in Linux but I don't remember the details.  I can just read
>> the solution that was picked in the code.
>>
>>
>>
>> All of this makes me wonder how are the LSMs protected against this
>> attack.
>>
>> Let's see the following LSMS implement brpm_set_creds:
>> tomoyo   - Abuses bprm_set_creds to call tomoyo_load_policy [ safe ]
>> smack    - Requires CAP_MAC_ADMIN to smack setxattrs        [ vulnerable? ]
>>            Uses those xattrs in smack_bprm_set_creds
>
> What is the concern? If the xattrs change after the check,
> the behavior should still be consistent.

The concern is that there are xattrs set on a #! script.  Someone
replaces the script after smack reads the xattr and sets bprm->cred but
before the interpreter reopens the script.

In short if there is one script with xattrs set. I can run any script as
if those xattrs were set on it.

I don't know the smack security model well enough to know if that
is a problem or not.  It looks like it may be a concern because smack
limits who can mess with it's security xattrs.

Eric