linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ariadne Conill <ariadne@dereferenced.org>
To: Rich Felker <dalias@libc.org>
Cc: Ariadne Conill <ariadne@dereferenced.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()
Date: Wed, 26 Jan 2022 11:37:58 -0600 (CST)	[thread overview]
Message-ID: <92b53c82-1588-36b3-b09b-e7c334e87e@dereferenced.org> (raw)
In-Reply-To: <20220126132729.GA7942@brightrain.aerifal.cx>

Hi,

On Wed, 26 Jan 2022, Rich Felker wrote:

> On Wed, Jan 26, 2022 at 04:39:47AM +0000, Ariadne Conill wrote:
>> The first argument to argv when used with execv family of calls is
>> required to be the name of the program being executed, per POSIX.
>
> That's not quite the story. The relevant text is a "should", meaning
> that to be "strictly conforming" an application has to follow the
> convention, but still can't assume its invoker did. (Note that most
> programs do not aim to be "strictly conforming"; it's not just the
> word strictly applied as an adjective to conforming, but a definition
> of its own imposing very stringent portability conditions beyond what
> the standard already imposes.) Moreover, POSIX (following ISO C, after
> this was changed from early C drafts) rejected making it a
> requirement. This is documented in the RATIONALE for execve:
>
>    Early proposals required that the value of argc passed to main()
>    be "one or greater". This was driven by the same requirement in
>    drafts of the ISO C standard. In fact, historical implementations
>    have passed a value of zero when no arguments are supplied to the
>    caller of the exec functions. This requirement was removed from
>    the ISO C standard and subsequently removed from this volume of
>    POSIX.1-2017 as well. The wording, in particular the use of the
>    word should, requires a Strictly Conforming POSIX Application to
>    pass at least one argument to the exec function, thus guaranteeing
>    that argc be one or greater when invoked by such an application.
>    In fact, this is good practice, since many existing applications
>    reference argv[0] without first checking the value of argc.
>
> Source: https://pubs.opengroup.org/onlinepubs/9699919799/functions/execve.html
>
> Note that despite citing itself as POSIX.1-2017 above, this is not a
> change in the 2017 edition; it's just the way they self-cite. As far
> as I can tell, the change goes back to prior to the first publication
> of the standard.

This was clarified in the v2 commit text.

>> By validating this in do_execveat_common(), we can prevent execution
>> of shellcode which invokes execv(2) family syscalls with argc < 1,
>> a scenario which is disallowed by POSIX, thus providing a mitigation
>> against CVE-2021-4034 and similar bugs in the future.
>>
>> The use of -EFAULT for this case is similar to other systems, such
>> as FreeBSD and OpenBSD.
>
> I don't like this choice of error, since in principle EFAULT should
> never happen when you haven't invoked memory-safety-violating UB.
> Something like EINVAL would be more appropriate. But if the existing
> practice for systems that do this is to use EFAULT, it's probably best
> to do the same thing.

It turns out that OpenBSD uses -EINVAL for this, see 
https://github.com/openbsd/src/commit/74212563870067f5b1e271876e1ec5a2fdf2f2e0

>
>> Interestingly, Michael Kerrisk opened an issue about this in 2008,
>> but there was no consensus to support fixing this issue then.
>> Hopefully now that CVE-2021-4034 shows practical exploitative use
>> of this bug in a shellcode, we can reconsider.
>
> I'm not really opposed to attempting to change this with consensus
> (like, actually proposing it on the Austin Group tracker), but a less
> invasive change would be just enforcing it for the case where exec is
> a privilege boundary (suid/sgid/caps). There's really no motivation
> for changing longstanding standard behavior in a
> non-privilege-boundary case.

It would be nice for the Austin Group to clarify this, but I think this is 
a "common sense" issue.  I don't think execve(2) with argc < 1 is 
"standard behavior" too, as many other systems outside Linux fail to 
execve(2) when argc < 1.

Ariadne

  parent reply	other threads:[~2022-01-26 17:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26  4:39 Ariadne Conill
2022-01-26  6:42 ` Kees Cook
2022-01-26  7:28   ` Kees Cook
2022-01-26 11:18     ` Ariadne Conill
2022-01-26 12:33       ` Heikki Kallasjoki
2022-01-26 23:57         ` Kees Cook
2022-01-27  0:20           ` Eric W. Biederman
2022-01-26 16:59     ` David Laight
2022-01-26 13:27 ` Rich Felker
2022-01-26 14:46   ` Christian Brauner
2022-01-26 17:37   ` Ariadne Conill [this message]
2022-02-01 20:54   ` hypervis0r
2022-01-26 15:02 Alexey Dobriyan
2022-01-27  0:00 ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=92b53c82-1588-36b3-b09b-e7c334e87e@dereferenced.org \
    --to=ariadne@dereferenced.org \
    --cc=dalias@libc.org \
    --cc=ebiederm@xmission.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).