Re: git: 773fa8cd136a - main - execve: disallow argc == 0
- In reply to: Cy Schubert : "Re: git: 773fa8cd136a - main - execve: disallow argc == 0"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 26 Jan 2022 20:05:01 UTC
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