Re: git: 773fa8cd136a - main - execve: disallow argc == 0

In message <202201261941.20QJfYf6038425@gitrepo.freebsd.org>, Kyle Evans 
writes
:
> The branch main has been updated by kevans:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=773fa8cd136a5775241c3e3a70f19976
> 33ebeedf
>
> commit 773fa8cd136a5775241c3e3a70f1997633ebeedf
> Author:     Kyle Evans <kevans@FreeBSD.org>
> AuthorDate: 2022-01-25 22:47:23 +0000
> Commit:     Kyle Evans <kevans@FreeBSD.org>
> CommitDate: 2022-01-26 19:40:27 +0000
>
>     execve: disallow argc == 0
>     
>     The manpage has contained the following verbiage on the matter for just
>     under 31 years:
>     
>     "At least one argument must be present in the array"
>     
>     Previous to this version, it had been prefaced with the weakening phrase
>     "By convention."
>     
>     Carry through and document it the rest of the way.  Allowing argc == 0
>     has been a source of security issues in the past, and it's hard to
>     imagine a valid use-case for allowing it.  Toss back EINVAL if we ended
>     up not copying in any args for *execve().
>     
>     The manpage change can be considered "Obtained from: OpenBSD"
>     
>     Reviewed by:    emaste, kib, markj (all previous version)
>     Differential Revision:  https://reviews.freebsd.org/D34045
> ---
>  lib/libc/sys/execve.2 | 5 ++++-
>  sys/kern/kern_exec.c  | 6 ++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/lib/libc/sys/execve.2 b/lib/libc/sys/execve.2
> index a8f5aa14854b..1abadba13d91 100644
> --- a/lib/libc/sys/execve.2
> +++ b/lib/libc/sys/execve.2
> @@ -28,7 +28,7 @@
>  .\"     @(#)execve.2	8.5 (Berkeley) 6/1/94
>  .\" $FreeBSD$
>  .\"
> -.Dd March 30, 2020
> +.Dd January 26, 2022
>  .Dt EXECVE 2
>  .Os
>  .Sh NAME
> @@ -273,6 +273,9 @@ Search permission is denied for a component of the path p
> refix.
>  The new process file is not an ordinary file.
>  .It Bq Er EACCES
>  The new process file mode denies execute permission.
> +.It Bq Er EINVAL
> +.Fa argv
> +did not contain at least one element.
>  .It Bq Er ENOEXEC
>  The new process file has the appropriate access
>  permission, but has an invalid magic number in its header.
> diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
> index 0494b73fc405..303c145689ae 100644
> --- a/sys/kern/kern_exec.c
> +++ b/sys/kern/kern_exec.c
> @@ -356,6 +356,12 @@ kern_execve(struct thread *td, struct image_args *args, 
> struct mac *mac_p,
>  	    exec_args_get_begin_envv(args) - args->begin_argv);
>  	AUDIT_ARG_ENVV(exec_args_get_begin_envv(args), args->envc,
>  	    args->endp - exec_args_get_begin_envv(args));
> +
> +	/* Must have at least one argument. */
> +	if (args->argc == 0) {
> +		exec_free_args(args);
> +		return (EINVAL);
> +	}
>  	return (do_execve(td, args, mac_p, oldvmspace));
>  }
>  
>

Thank you. I think this might help me track down a bug in a port.

Can we MFC this at some point?


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.