Ted Unangst <t...@tedunangst.com> wrote:
> Ted Unangst wrote:
> > Bryan Steele wrote:
> > > It is not possible to unveil(2) all arguments passed to file(1), as this
> > > would require walking *argv. Instead, we can unveil("/", "r") to permit
> > > readonly access to the entire filesystem, while restricting all execute
> > > write, and create operations.
> >
> > Why not? Because of the limit? We can still try unveil up to a certain
> > limit.
> >
> > > This only provides some additional early protection for the parent, as
> > > the privsep magic(5) parser already pledged tightly.
> > >
> > > It might be possible to use pledge instead, but this since this process
> > > doesn't do much more than opening files and passing descriptors, unveil
> > > alone should be enough..
> >
> > I think if we want to enforce read only access, pledge is still the way to
> > go.
> >
> > This seems to work.
>
> oops, forgot the error checking for some unveil calls. this is better.
>
>
> Index: file.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 file.c
> --- file.c 15 Jan 2018 19:45:51 -0000 1.66
> +++ file.c 4 Jan 2019 01:24:47 -0000
> @@ -168,6 +168,19 @@ main(int argc, char **argv)
> } else if (argc == 0)
> usage();
>
> + if (argc < 64) {
> + if (unveil("/etc/magic", "r") == -1)
> + err(1, "unveil");
> + for (idx = 0; idx < argc; idx++)
> + if (unveil(argv[idx], "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> + }
> +
> + if (pledge("stdio rpath getpw recvfd sendfd id proc", NULL) == -1)
> + err(1, "pledge");
> +
> magicfp = NULL;
> if (geteuid() != 0 && !issetugid()) {
> home = getenv("HOME");
>
I absolutely do not OK this.
You are abusing kernel resources.
