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

On Wed, Jan 26, 2022 at 2:02 PM Cy Schubert <Cy.Schubert@cschubert.com> wrote:
>
> 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?
>

I'll probably MFC these in a week or two, I can't imagine it will
cause any real damage.

Thanks,

Kyle Evans