This is an example of better to start at just hoisting the code that
opens the many fds and put them all inside open_files(). After that it's
just a matter of calling pledge("stdio") and we are done.
Of course that after this is done we can still make a list of all the files
we need to open and unveil them, but not the way it's done here.
Once I get back home from $DAYJOB I'll try to have a look at this.
On 08:04 Tue 25 Sep , Theo de Raadt wrote:
> Theo de Raadt <dera...@openbsd.org> wrote:
>
> > Michael Mikonos <m...@ii.net> wrote:
> >
> > > On Mon, Sep 24, 2018 at 10:53:47PM -0600, Theo de Raadt wrote:
> > > > Ugh. A diff which doens't check error returns. Averting my gaze
> > > > is similar to "no way". Hope you have another quarter, because you
> > > > need to try again
> > >
> > > Oops... new coin inserted. I decided to create a fatal_perror()
> > > function which behaves like perror(). yacc's error.c didn't seem
> > > to have anything like that. Now if either pledge() or unveil()
> > > fails we see the appropriate error string instead of always seeing
> > > "invalid arguments" for pledge().
> >
> > I don't understand parts of the diff.
> >
> > > Index: defs.h
> > > ===================================================================
> > > RCS file: /cvs/src/usr.bin/yacc/defs.h,v
> > > retrieving revision 1.18
> > > diff -u -p -u -r1.18 defs.h
> > > --- defs.h 2 Dec 2014 15:56:22 -0000 1.18
> > > +++ defs.h 25 Sep 2018 05:34:27 -0000
> > > @@ -307,6 +307,7 @@ extern void closure(short *, int);
> > > extern void finalize_closure(void);
> > >
> > > extern __dead void fatal(char *);
> > > +extern __dead void fatal_perror(char *);
> > >
> > > extern void reflexive_transitive_closure(unsigned *, int);
> > > extern __dead void done(int);
> > > Index: error.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.bin/yacc/error.c,v
> > > retrieving revision 1.14
> > > diff -u -p -u -r1.14 error.c
> > > --- error.c 8 Mar 2014 01:05:39 -0000 1.14
> > > +++ error.c 25 Sep 2018 05:34:27 -0000
> > > @@ -35,6 +35,8 @@
> > >
> > > /* routines for printing error messages */
> > >
> > > +#include <errno.h>
> > > +
> > > #include "defs.h"
> > >
> > >
> > > @@ -45,6 +47,12 @@ fatal(char *msg)
> > > done(2);
> > > }
> > >
> > > +void
> > > +fatal_perror(char *msg)
> > > +{
> > > + fprintf(stderr, "%s: %s\n", msg, strerror(errno));
> > > + done(2);
> > > +}
> > >
> > > void
> > > no_space(void)
> > > Index: main.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.bin/yacc/main.c,v
> > > retrieving revision 1.29
> > > diff -u -p -u -r1.29 main.c
> > > --- main.c 25 May 2017 20:11:03 -0000 1.29
> > > +++ main.c 25 Sep 2018 05:34:27 -0000
> > > @@ -305,10 +305,14 @@ open_files(void)
> > > create_file_names();
> > >
> > > if (input_file == 0) {
> > > + if (unveil(input_file_name, "r") == -1)
> > > + fatal_perror("unveil");
> > > input_file = fopen(input_file_name, "r");
> >
> > At this point, all files are allowed.
> > So you restrict it to one.
> > Then you open it.
> > You won't open it again.
> > Why does it remain on the permitted open list?
> > Why not just open it, and not have it on the permitted open list?
> >
> > Meanwhile, unveil hasn't been blocked. So if someone finds a bug
> > at this point which gives them control, they can continue calling
> > unveil, and get full filesystem access back.
>
>
> As another way of explaining, the logic you created goes like this
>
> allow only this file
> open it
> do some more calculation
> oh wait, allow this file also!
> open it
> do some more calculation
> OH WAIT, here is another file to open...
>
