cat(1): add more restrictive pledge(2)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

cat(1): add more restrictive pledge(2)

tempmail42
I have to say I'm only a beginner to C but hopefully my patch is
good.

This patch adds a second and more restrictive pledge (only "stdio"
instead of "stdio rpath") after the getopt loop if there is no
input file or if the input file is "-" (stdin) or a sequence of
repeated instances of "-". It doesn't move argv past the last "-",
and doesn't pledge if it runs into an input file other than "-".

I've compiled it and tested it with ktrace(1) and kdump(1) and it
appears to work as expected and pledge correctly with at least
these invocations:
$ echo test | ./cat -uv     # pledge("stdio", NULL);
$ echo test | ./cat -uv -   # pledge("stdio", NULL);
$ echo test | ./cat         # pledge("stdio", NULL);
$ echo test | ./cat - - -   # pledge("stdio", NULL);
$ echo test | ./cat -       # pledge("stdio", NULL);
$ echo test | ./cat - cat.c # pledge("stdio rpath", NULL);
$ echo test | ./cat cat.c - # pledge("stdio rpath", NULL);



Index: bin/cat/cat.c
===================================================================
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 cat.c
--- bin/cat/cat.c 28 Jun 2019 13:34:58 -0000 1.27
+++ bin/cat/cat.c 30 Jul 2020 23:21:14 -0000
@@ -94,7 +94,26 @@ main(int argc, char *argv[])
     "usage: %s [-benstuv] [file ...]\n", __progname);
  return 1;
  }
+ argc -= optind;
  argv += optind;
+
+ if (argc) {
+ if (!strcmp(*argv, "-")) {
+ do {
+ if (argc == 1) {
+ if (pledge("stdio", NULL) == -1)
+ err(1, "pledge");
+ argc--, argv++;
+ break;
+ } else
+ argc--, argv++;
+ } while (argc && !strcmp(*argv, "-"));
+ argc++, argv--;
+ }
+ } else {
+ if (pledge("stdio", NULL) == -1)
+ err(1, "pledge");
+ }

  if (bflag || eflag || nflag || sflag || tflag || vflag)
  cook_args(argv);

Reply | Threaded
Open this post in threaded view
|

Re: cat(1): add more restrictive pledge(2)

Stuart Henderson
On 2020/07/31 00:07, [hidden email] wrote:

> I have to say I'm only a beginner to C but hopefully my patch is
> good.
>
> This patch adds a second and more restrictive pledge (only "stdio"
> instead of "stdio rpath") after the getopt loop if there is no
> input file or if the input file is "-" (stdin) or a sequence of
> repeated instances of "-". It doesn't move argv past the last "-",
> and doesn't pledge if it runs into an input file other than "-".
>
> I've compiled it and tested it with ktrace(1) and kdump(1) and it
> appears to work as expected and pledge correctly with at least
> these invocations:
> $ echo test | ./cat -uv     # pledge("stdio", NULL);
> $ echo test | ./cat -uv -   # pledge("stdio", NULL);
> $ echo test | ./cat         # pledge("stdio", NULL);
> $ echo test | ./cat - - -   # pledge("stdio", NULL);
> $ echo test | ./cat -       # pledge("stdio", NULL);
> $ echo test | ./cat - cat.c # pledge("stdio rpath", NULL);
> $ echo test | ./cat cat.c - # pledge("stdio rpath", NULL);
>
>
>
> Index: bin/cat/cat.c
> ===================================================================
> RCS file: /cvs/src/bin/cat/cat.c,v
> retrieving revision 1.27
> diff -u -p -u -p -r1.27 cat.c
> --- bin/cat/cat.c 28 Jun 2019 13:34:58 -0000 1.27
> +++ bin/cat/cat.c 30 Jul 2020 23:21:14 -0000
> @@ -94,7 +94,26 @@ main(int argc, char *argv[])
>      "usage: %s [-benstuv] [file ...]\n", __progname);
>   return 1;
>   }
> + argc -= optind;
>   argv += optind;
> +
> + if (argc) {
> + if (!strcmp(*argv, "-")) {
> + do {
> + if (argc == 1) {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + argc--, argv++;
> + break;
> + } else
> + argc--, argv++;
> + } while (argc && !strcmp(*argv, "-"));
> + argc++, argv--;
> + }
> + } else {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + }
>
>   if (bflag || eflag || nflag || sflag || tflag || vflag)
>   cook_args(argv);
>

The improvement is fairly small; cat doesn't have network access or the
ability to write files with the previous pledge. Is this worth the
considerable extra complexity?

It's hard to get a feel for whether the argc/argv manipulation is correct.

Reply | Threaded
Open this post in threaded view
|

Re: cat(1): add more restrictive pledge(2)

Theo de Raadt-2
As a general rule, pledge uses isn't supposed to make programs
more complicated.

Stuart Henderson <[hidden email]> wrote:

> On 2020/07/31 00:07, [hidden email] wrote:
> > I have to say I'm only a beginner to C but hopefully my patch is
> > good.
> >
> > This patch adds a second and more restrictive pledge (only "stdio"
> > instead of "stdio rpath") after the getopt loop if there is no
> > input file or if the input file is "-" (stdin) or a sequence of
> > repeated instances of "-". It doesn't move argv past the last "-",
> > and doesn't pledge if it runs into an input file other than "-".
> >
> > I've compiled it and tested it with ktrace(1) and kdump(1) and it
> > appears to work as expected and pledge correctly with at least
> > these invocations:
> > $ echo test | ./cat -uv     # pledge("stdio", NULL);
> > $ echo test | ./cat -uv -   # pledge("stdio", NULL);
> > $ echo test | ./cat         # pledge("stdio", NULL);
> > $ echo test | ./cat - - -   # pledge("stdio", NULL);
> > $ echo test | ./cat -       # pledge("stdio", NULL);
> > $ echo test | ./cat - cat.c # pledge("stdio rpath", NULL);
> > $ echo test | ./cat cat.c - # pledge("stdio rpath", NULL);
> >
> >
> >
> > Index: bin/cat/cat.c
> > ===================================================================
> > RCS file: /cvs/src/bin/cat/cat.c,v
> > retrieving revision 1.27
> > diff -u -p -u -p -r1.27 cat.c
> > --- bin/cat/cat.c 28 Jun 2019 13:34:58 -0000 1.27
> > +++ bin/cat/cat.c 30 Jul 2020 23:21:14 -0000
> > @@ -94,7 +94,26 @@ main(int argc, char *argv[])
> >      "usage: %s [-benstuv] [file ...]\n", __progname);
> >   return 1;
> >   }
> > + argc -= optind;
> >   argv += optind;
> > +
> > + if (argc) {
> > + if (!strcmp(*argv, "-")) {
> > + do {
> > + if (argc == 1) {
> > + if (pledge("stdio", NULL) == -1)
> > + err(1, "pledge");
> > + argc--, argv++;
> > + break;
> > + } else
> > + argc--, argv++;
> > + } while (argc && !strcmp(*argv, "-"));
> > + argc++, argv--;
> > + }
> > + } else {
> > + if (pledge("stdio", NULL) == -1)
> > + err(1, "pledge");
> > + }
> >
> >   if (bflag || eflag || nflag || sflag || tflag || vflag)
> >   cook_args(argv);
> >
>
> The improvement is fairly small; cat doesn't have network access or the
> ability to write files with the previous pledge. Is this worth the
> considerable extra complexity?
>
> It's hard to get a feel for whether the argc/argv manipulation is correct.
>