[PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Ariadne Conill]

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.

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.

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.

Signed-off-by: Ariadne Conill <ariadne@dereferenced.org>
---
 fs/exec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/exec.c b/fs/exec.c
index 79f2c9483302..de0b832473ed 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1897,8 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename,
 	}
 
 	retval = count(argv, MAX_ARG_STRINGS);
-	if (retval < 0)
+	if (retval < 1) {
+		retval = -EFAULT;
 		goto out_free;
+	}
 	bprm->argc = retval;
 
 	retval = count(envp, MAX_ARG_STRINGS);
-- 
2.34.1

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Kees Cook]

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.
> 
> 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.
> 
> 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.
> 
> Signed-off-by: Ariadne Conill <ariadne@dereferenced.org>

Yup. Agreed. For context:
https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt

> ---
>  fs/exec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 79f2c9483302..de0b832473ed 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1897,8 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename,
>  	}
>  
>  	retval = count(argv, MAX_ARG_STRINGS);
> -	if (retval < 0)
> +	if (retval < 1) {
> +		retval = -EFAULT;
>  		goto out_free;
> +	}

There shouldn't be anything legitimate actually doing this in userspace.

-Kees

>  	bprm->argc = retval;
>  
>  	retval = count(envp, MAX_ARG_STRINGS);
> -- 
> 2.34.1
> 

-- 
Kees Cook

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Kees Cook]

On January 25, 2022 10:42:41 PM PST, Kees Cook <keescook@chromium.org> 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.
>> 
>> 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.
>> 
>> Interestingly, Michael Kerrisk opened an issue about this in 2008,

For v2 please include a URL for this. I assume you mean this one?
https://bugzilla.kernel.org/show_bug.cgi?id=8408

>> 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.
>> 
>> Signed-off-by: Ariadne Conill <ariadne@dereferenced.org>
>
>Yup. Agreed. For context:
>https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt
>
>> ---
>>  fs/exec.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/exec.c b/fs/exec.c
>> index 79f2c9483302..de0b832473ed 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -1897,8 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename,
>>  	}
>>  
>>  	retval = count(argv, MAX_ARG_STRINGS);
>> -	if (retval < 0)
>> +	if (retval < 1) {
>> +		retval = -EFAULT;
>>  		goto out_free;
>> +	}

Actually, no, this needs to be more carefully special-cased to avoid masking error returns from count(). (e.g. -E2BIG would vanish with this patch.)

Perhaps just add:

if (retval == 0) {
        retval = -EFAULT;
        goto out_free;
}

>
>There shouldn't be anything legitimate actually doing this in userspace.

I spoke too soon.

Unfortunately, this is not the case:
https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0

Lots of stuff likes to do:
execve(path, NULL, NULL);

Do these things depend on argc==0 would be my next question...

>
>-Kees
>
>>  	bprm->argc = retval;
>>  
>>  	retval = count(envp, MAX_ARG_STRINGS);
>> -- 
>> 2.34.1
>> 
>

-- 
Kees Cook

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Ariadne Conill]

Hi,

On Tue, 25 Jan 2022, Kees Cook wrote:

>
>
> On January 25, 2022 10:42:41 PM PST, Kees Cook <keescook@chromium.org> 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.
>>>
>>> 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.
>>>
>>> Interestingly, Michael Kerrisk opened an issue about this in 2008,
>
> For v2 please include a URL for this. I assume you mean this one?
> https://bugzilla.kernel.org/show_bug.cgi?id=8408

Yes, that's the one.  I honestly need to rewrite that commit message 
anyway.

>>> 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.
>>>
>>> Signed-off-by: Ariadne Conill <ariadne@dereferenced.org>
>>
>> Yup. Agreed. For context:
>> https://www.qualys.com/2022/01/25/cve-2021-4034/pwnkit.txt
>>
>>> ---
>>>  fs/exec.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/exec.c b/fs/exec.c
>>> index 79f2c9483302..de0b832473ed 100644
>>> --- a/fs/exec.c
>>> +++ b/fs/exec.c
>>> @@ -1897,8 +1897,10 @@ static int do_execveat_common(int fd, struct filename *filename,
>>>  	}
>>>
>>>  	retval = count(argv, MAX_ARG_STRINGS);
>>> -	if (retval < 0)
>>> +	if (retval < 1) {
>>> +		retval = -EFAULT;
>>>  		goto out_free;
>>> +	}
>
> Actually, no, this needs to be more carefully special-cased to avoid masking error returns from count(). (e.g. -E2BIG would vanish with this patch.)
>
> Perhaps just add:
>
> if (retval == 0) {
>        retval = -EFAULT;
>        goto out_free;
> }

Alright.  I will do that in v2.

>>
>> There shouldn't be anything legitimate actually doing this in userspace.
>
> I spoke too soon.
>
> Unfortunately, this is not the case:
> https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0
>
> Lots of stuff likes to do:
> execve(path, NULL, NULL);
>
> Do these things depend on argc==0 would be my next question...

I looked at these, and these seem to basically be lazily-written test 
cases which should be fixed.  I didn't see any example of real-world 
applications doing this.  As noted in some of the test cases, there are 
comments like "Solaris doesn't support this," etc.

So I think having this as a config option at the very least makes a lot of 
sense.  If users really need to run legacy code where execv() works with 
argc < 1, then they could just run a kernel that allows that nonsense, 
just like how Linux doesn't necessarily support the old a.out binary 
format today, unless it is enabled.

Ariadne

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Heikki Kallasjoki]

On Wed, Jan 26, 2022 at 05:18:58AM -0600, Ariadne Conill wrote:
> On Tue, 25 Jan 2022, Kees Cook wrote:
> > Lots of stuff likes to do:
> > execve(path, NULL, NULL);
> 
> I looked at these, and these seem to basically be lazily-written test cases
> which should be fixed.  I didn't see any example of real-world applications
> doing this.  As noted in some of the test cases, there are comments like
> "Solaris doesn't support this," etc.

See also the (small) handful of instances of `execlp(cmd, NULL);` out
there, which I imagine would start to fail:
https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0

Two of the hits (ispell, nauty) would seem to be non-test use cases.

As an aside, saying POSIX "disallows" argc == 0 might be overstating it
a little. As far as I can tell (quotes below), while a Strictly
Conforming POSIX Application must provide argc >= 1 to a program it
executes, the argc == 0 case isn't entirely disallowed.

https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/V1_chap01.html

"should -- describes a feature or behavior that is recommended but not
mandatory. An application should not rely on the existence of the
feature or behavior."

https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/execve.html

"The value in argv[0] *should* point to a filename string that is
associated with the process --" (emphasis added)

"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."

Just to be clear, not disputing the part that disallowing `argc == 0`
would be a reasonable idea, or claiming that there's a valid use case.
Just the part where POSIX would *require* the system to disallow this.

-- 
Heikki Kallasjoki

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Kees Cook]

On Wed, Jan 26, 2022 at 12:33:39PM +0000, Heikki Kallasjoki wrote:
> On Wed, Jan 26, 2022 at 05:18:58AM -0600, Ariadne Conill wrote:
> > On Tue, 25 Jan 2022, Kees Cook wrote:
> > > Lots of stuff likes to do:
> > > execve(path, NULL, NULL);
> > 
> > I looked at these, and these seem to basically be lazily-written test cases
> > which should be fixed.  I didn't see any example of real-world applications
> > doing this.  As noted in some of the test cases, there are comments like
> > "Solaris doesn't support this," etc.
> 
> See also the (small) handful of instances of `execlp(cmd, NULL);` out
> there, which I imagine would start to fail:
> https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0
> 
> Two of the hits (ispell, nauty) would seem to be non-test use cases.

Ah yeah, I've added this to the Issue tracker:
https://github.com/KSPP/linux/issues/176

-- 
Kees Cook

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Eric W. Biederman]

Kees Cook <keescook@chromium.org> writes:

> On Wed, Jan 26, 2022 at 12:33:39PM +0000, Heikki Kallasjoki wrote:
>> On Wed, Jan 26, 2022 at 05:18:58AM -0600, Ariadne Conill wrote:
>> > On Tue, 25 Jan 2022, Kees Cook wrote:
>> > > Lots of stuff likes to do:
>> > > execve(path, NULL, NULL);
>> > 
>> > I looked at these, and these seem to basically be lazily-written test cases
>> > which should be fixed.  I didn't see any example of real-world applications
>> > doing this.  As noted in some of the test cases, there are comments like
>> > "Solaris doesn't support this," etc.
>> 
>> See also the (small) handful of instances of `execlp(cmd, NULL);` out
>> there, which I imagine would start to fail:
>> https://codesearch.debian.net/search?q=execlp%3F%5Cs*%5C%28%5B%5E%2C%5D%2B%2C%5Cs*NULL&literal=0
>> 
>> Two of the hits (ispell, nauty) would seem to be non-test use cases.
>
> Ah yeah, I've added this to the Issue tracker:
> https://github.com/KSPP/linux/issues/176

I just took a slightly deeper look at these.

There are two patterns found by that search.

-  execlp("/proc/self/exec", NULL)

    Which in both both the proot and care packages is a testt that
    deliberately loops and checks to see if it can generate "argc == 0".

    That is the case where changing argc == 0 into { "", NULL }
    will loop forever.

    For that test failing to exec the "argc == 0" will cause a test
    failure but for a security issue that seems a reasonable thing
    to do to a test.

- execlp(MACRO, NULL)

    The macro happens to contain commas so what looks via inspection
    will generate an application run with "argc == 0" actually
    does not.

Eric

RE: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[David Laight]

From: Kees Cook
> Sent: 26 January 2022 07:28
...
> >
> >There shouldn't be anything legitimate actually doing this in userspace.
> 
> I spoke too soon.
> 
> Unfortunately, this is not the case:
> https://codesearch.debian.net/search?q=execve%5C+*%5C%28%5B%5E%2C%5D%2B%2C+*NULL&literal=0
> 
> Lots of stuff likes to do:
> execve(path, NULL, NULL);
> 
> Do these things depend on argc==0 would be my next question...

What about ensuring that argv[0] and argv[1] are always present
and both NULL when argc is 0?

Then programs that just scan from argv[1] until they get a NULL
will always be fine.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Rich Felker]

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.

> 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.

> 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.

Rich

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Christian Brauner]

On Wed, Jan 26, 2022 at 08:27:30AM -0500, 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.
> 
> > 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.
> 
> > 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.

Agreed. If we do this at all then this has way less regression potential.

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Ariadne Conill]

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

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[hypervis0r]

> 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.

I don't really see it as a matter of "maintaining standard behavior".

there are very little uses for this ABI feature to be present and only 
serves to make applications harder to port between Linux and other *nix 
systems. The pros (major vulnerabilities like CVE-2021-4034) outweigh 
the cons (minor userland ABI change that only affects shellcode on 
shell-storm.org) in this particular scenario, and I am all for this patch.

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Alexey Dobriyan]

>	execve("...", NULL, NULL);

I personally wrote a program which relies on execve(NULL) to succeed.
It wasn't an exploit, it was test program against IMA-like kernel
"only whitelisted executables can run" feature.

Test copies and "corrupts" itself by appending \0 to the end, then tries
to reexec itself with execve("/proc/self/exe", NULL, NULL);
main() if run with argc==0 exits with specific error code.

Appending \0 breaks checksum so working kernel protection scheme must
not allow it, therefore if execve(NULL) succeeded, than the parent
process doing test hard fails.

Also appending \0 doesn't break ELF structure. In other words,
if executable A is working (and it is working because it is running)
then A||\0 is valid executable as well and will run too.

This is independent from filesystem layout, libc, kernel, dynamic
libraries, compile options and what not.

Now QNX doesn't allow execve(NULL) and I don't remember if I changed it
to the next simplest variant and I don't work anymore at that company,
so I can't check :^)

	execve("/proc/self/exe", (char*[]){"Alexey", NULL}, NULL);

P.S.:

	> tptacek 5 minutes ago | root | parent | next [–]
	> There is not.

	Yes, there is!

Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()

[Kees Cook]

On Wed, Jan 26, 2022 at 06:02:25PM +0300, Alexey Dobriyan wrote:
> >	execve("...", NULL, NULL);
> 
> I personally wrote a program which relies on execve(NULL) to succeed.
> It wasn't an exploit, it was test program against IMA-like kernel
> "only whitelisted executables can run" feature.
> 
> Test copies and "corrupts" itself by appending \0 to the end, then tries
> to reexec itself with execve("/proc/self/exe", NULL, NULL);
> main() if run with argc==0 exits with specific error code.
> 
> Appending \0 breaks checksum so working kernel protection scheme must
> not allow it, therefore if execve(NULL) succeeded, than the parent
> process doing test hard fails.
> 
> Also appending \0 doesn't break ELF structure. In other words,
> if executable A is working (and it is working because it is running)
> then A||\0 is valid executable as well and will run too.
> 
> This is independent from filesystem layout, libc, kernel, dynamic
> libraries, compile options and what not.
> 
> Now QNX doesn't allow execve(NULL) and I don't remember if I changed it
> to the next simplest variant and I don't work anymore at that company,
> so I can't check :^)
> 
> 	execve("/proc/self/exe", (char*[]){"Alexey", NULL}, NULL);

One of the various suggestions was to inject { path, NULL } when argc=0.

Given that execve(path, NULL, ...) is being used at least a little,
hopefully there is nothing that depends on argc==0... :P

-- 
Kees Cook