Re: yacc + unveil

2018-10-07 Thread Michael Mikonos
Hello,

Forwarding a newer patch that I came up with. This time unveil()s are
done before pledge() so no subsequent pledge() is needed to remove
the unveil promise.

* temporary files are created & unlinked in /tmp, so unveil the directory
* output_file_name is either -o PATH or the default of CWD/y.tab.c;
  open it for write
* code_file_name is CWD/y.code.c if -r option is set, otherwise
  code_file_name==output_file_name; add separate unveil for -r case
* input_file_name is the input path, or empty string for filename of "-";
  open it for read if not an empty string
* verbose_file_name is CWD/y.output if -v option is set, otherwise NULL;
  open it for write if not NULL
* defines_file_name is a path in the same directory as output_file_name
  if option -d is set, otherwise NULL; open it for write if not NULL

getargs() and create_file_names() are responsible for setting the
global variables for the various paths based on argv, so move them to
the top. create_file_names() is being lifted from open_files() which
actually performs fopen().


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 -  1.29
+++ main.c  1 Oct 2018 07:11:47 -
@@ -34,6 +34,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -302,8 +303,6 @@ open_files(void)
 {
int fd;
 
-   create_file_names();
-
if (input_file == 0) {
input_file = fopen(input_file_name, "r");
if (input_file == 0)
@@ -346,11 +345,34 @@ open_files(void)
 int
 main(int argc, char *argv[])
 {
+   getargs(argc, argv);
+   create_file_names();
+
+   if (unveil(_PATH_TMP, "wrc") == -1)
+   err(1, "unveil");
+   if (unveil(output_file_name, "wc") == -1)
+   err(1, "unveil");
+   if (code_file_name != output_file_name) {
+   if (unveil(code_file_name, "wc") == -1)
+   err(1, "unveil");
+   }
+   if (verbose_file_name != NULL) {
+   if (unveil(verbose_file_name, "wc") == -1)
+   err(1, "unveil");
+   }
+   if (defines_file_name != NULL) {
+   if (unveil(defines_file_name, "wc") == -1)
+   err(1, "unveil");
+   }
+   if (strlen(input_file_name) > 0) {
+   if (unveil(input_file_name, "r") == -1)
+   err(1, "unveil");
+   }
+
if (pledge("stdio rpath wpath cpath", NULL) == -1)
fatal("pledge: invalid arguments");
 
set_signals();
-   getargs(argc, argv);
open_files();
reader();
lr0();



Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 11:42:26PM +0800, Michael Mikonos wrote:
> On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote:
> > On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> > > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > > > 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.
> > > 
> > > After open_files() the wpath pledge can be dropped. rpath is still
> > > needed because /tmp files are reopened for read in output(). cpath
> > > is needed because /tmp files are unlinked at the end. This patch
> > > adds a pledge call, but is it better to just move the first pledge()
> > > down?
> > > 
> > 
> > you could try with the "tmppath" promise. I will allow opening/creating
> > files on /tmp and unlinking them (but not sure it will cover all yacc
> > need as it is designed for mkstemp(3) family). Unveil for such
> > operations are fine too, without explicit unveil(2) call.
> > 
> 
> Ah, I see what you mean. pledging "tmppath" is kind of like unveil
> because the allowed operations only work under /tmp.
> It's possible to do this after calling open_files() because the only
> files (re)opened later are in /tmp.

My description was wrong. tmppath allows unlink of /tmp files at the
end. rpath is still needed to reopen the /tmp files.

> 
> 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.c25 May 2017 20:11:03 -  1.29
> +++ main.c25 Sep 2018 15:38:18 -
> @@ -346,12 +346,16 @@ open_files(void)
>  int
>  main(int argc, char *argv[])
>  {
> - if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + if (pledge("stdio rpath wpath cpath tmppath", NULL) == -1)
>   fatal("pledge: invalid arguments");
>  
>   set_signals();
>   getargs(argc, argv);
>   open_files();
> +
> + if (pledge("stdio rpath tmppath", NULL) == -1)
> + fatal("pledge: invalid arguments");
> +
>   reader();
>   lr0();
>   lalr();



Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote:
> On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > > 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.
> > 
> > After open_files() the wpath pledge can be dropped. rpath is still
> > needed because /tmp files are reopened for read in output(). cpath
> > is needed because /tmp files are unlinked at the end. This patch
> > adds a pledge call, but is it better to just move the first pledge()
> > down?
> > 
> 
> you could try with the "tmppath" promise. I will allow opening/creating
> files on /tmp and unlinking them (but not sure it will cover all yacc
> need as it is designed for mkstemp(3) family). Unveil for such
> operations are fine too, without explicit unveil(2) call.
> 

Ah, I see what you mean. pledging "tmppath" is kind of like unveil
because the allowed operations only work under /tmp.
It's possible to do this after calling open_files() because the only
files (re)opened later are in /tmp.


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 -  1.29
+++ main.c  25 Sep 2018 15:38:18 -
@@ -346,12 +346,16 @@ open_files(void)
 int
 main(int argc, char *argv[])
 {
-   if (pledge("stdio rpath wpath cpath", NULL) == -1)
+   if (pledge("stdio rpath wpath cpath tmppath", NULL) == -1)
fatal("pledge: invalid arguments");
 
set_signals();
getargs(argc, argv);
open_files();
+
+   if (pledge("stdio rpath tmppath", NULL) == -1)
+   fatal("pledge: invalid arguments");
+
reader();
lr0();
lalr();



Re: yacc + unveil

2018-09-25 Thread Sebastien Marie
On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > 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.
> 
> After open_files() the wpath pledge can be dropped. rpath is still
> needed because /tmp files are reopened for read in output(). cpath
> is needed because /tmp files are unlinked at the end. This patch
> adds a pledge call, but is it better to just move the first pledge()
> down?
> 

you could try with the "tmppath" promise. I will allow opening/creating
files on /tmp and unlinking them (but not sure it will cover all yacc
need as it is designed for mkstemp(3) family). Unveil for such
operations are fine too, without explicit unveil(2) call.

-- 
Sebastien Marie



Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> 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.

After open_files() the wpath pledge can be dropped. rpath is still
needed because /tmp files are reopened for read in output(). cpath
is needed because /tmp files are unlinked at the end. This patch
adds a pledge call, but is it better to just move the first pledge()
down?


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 -  1.29
+++ main.c  25 Sep 2018 15:07:35 -
@@ -352,6 +352,10 @@ main(int argc, char *argv[])
set_signals();
getargs(argc, argv);
open_files();
+
+   if (pledge("stdio rpath cpath", NULL) == -1)
+   fatal("pledge: invalid arguments");
+
reader();
lr0();
lalr();



Re: yacc + unveil

2018-09-25 Thread Ricardo Mestre
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  wrote:
> 
> > Michael Mikonos  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.h2 Dec 2014 15:56:22 -   1.18
> > > +++ defs.h25 Sep 2018 05:34:27 -
> > > @@ -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 -   1.14
> > > +++ error.c   25 Sep 2018 05:34:27 -
> > > @@ -35,6 +35,8 @@
> > >  
> > >  /* routines for printing error messages  */
> > >  
> > > +#include 
> > > +
> > >  #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.c25 May 2017 20:11:03 -  1.29
> > > +++ main.c25 Sep 2018 05:34:27 -
> > > @@ -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...
> 



Re: yacc + unveil

2018-09-25 Thread Theo de Raadt
Theo de Raadt  wrote:

> Michael Mikonos  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 -   1.18
> > +++ defs.h  25 Sep 2018 05:34:27 -
> > @@ -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 -   1.14
> > +++ error.c 25 Sep 2018 05:34:27 -
> > @@ -35,6 +35,8 @@
> >  
> >  /* routines for printing error messages  */
> >  
> > +#include 
> > +
> >  #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 -  1.29
> > +++ main.c  25 Sep 2018 05:34:27 -
> > @@ -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...



Re: yacc + unveil

2018-09-25 Thread Theo de Raadt
Michael Mikonos  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.h2 Dec 2014 15:56:22 -   1.18
> +++ defs.h25 Sep 2018 05:34:27 -
> @@ -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 -   1.14
> +++ error.c   25 Sep 2018 05:34:27 -
> @@ -35,6 +35,8 @@
>  
>  /* routines for printing error messages  */
>  
> +#include 
> +
>  #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.c25 May 2017 20:11:03 -  1.29
> +++ main.c25 Sep 2018 05:34:27 -
> @@ -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.

>   if (input_file == 0)
>   open_error(input_file_name);
>   }
> + if (unveil("/tmp", "crw") == -1)
> + fatal_perror("unveil");
>   fd = mkstemp(action_file_name);

Same concern.

>   if (fd == -1 || (action_file = fdopen(fd, "w")) == NULL)
>   open_error(action_file_name);
> @@ -318,11 +322,15 @@ open_files(void)
>   open_error(text_file_name);
>  
>   if (vflag) {
> + if (unveil(verbose_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   verbose_file = fopen(verbose_file_name, "w");

Same concern.

>   if (verbose_file == 0)
>   open_error(verbose_file_name);
>   }
>   if (dflag) {
> + if (unveil(defines_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   defines_file = fopen(defines_file_name, "w");

Same concern.

>   if (defines_file == NULL)
>   open_write_error(defines_file_name);
> @@ -330,24 +338,30 @@ open_files(void)
>   if (fd == -1 || (union_file = fdopen(fd, "w")) == NULL)
>   open_error(union_file_name);
>   }
> + if (unveil(output_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   output_file = fopen(output_file_name, "w");

Same concern.

>   if (output_file == 0)
>   open_error(output_file_name);
>  
>   if (rflag) {
> + if (unveil(code_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   code_file = fopen(code_file_name, "w");

Same concern.

The best approach for using unveil should be more similar to pledge -
calculate the valid security ruleset early on.  Hoist initialization
earlier, and hoist security initialization code even earlier if possible.

That means, try to seperate the generation of the paths way up front,
unveil them, then pledge without unveil or unveil(NULL,NULL), and then
let the post-initialization part of the program continue.

>   if (code_file == 0)
>   open_error(code_file_name);
>   } else
>   code_file = output_file;
> + if (unveil(NULL, NULL) == -1)
> 

Re: yacc + unveil

2018-09-24 Thread Michael Mikonos
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().


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 -   1.18
+++ defs.h  25 Sep 2018 05:34:27 -
@@ -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 -   1.14
+++ error.c 25 Sep 2018 05:34:27 -
@@ -35,6 +35,8 @@
 
 /* routines for printing error messages  */
 
+#include 
+
 #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 -  1.29
+++ main.c  25 Sep 2018 05:34:27 -
@@ -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");
if (input_file == 0)
open_error(input_file_name);
}
+   if (unveil("/tmp", "crw") == -1)
+   fatal_perror("unveil");
fd = mkstemp(action_file_name);
if (fd == -1 || (action_file = fdopen(fd, "w")) == NULL)
open_error(action_file_name);
@@ -318,11 +322,15 @@ open_files(void)
open_error(text_file_name);
 
if (vflag) {
+   if (unveil(verbose_file_name, "cw") == -1)
+   fatal_perror("unveil");
verbose_file = fopen(verbose_file_name, "w");
if (verbose_file == 0)
open_error(verbose_file_name);
}
if (dflag) {
+   if (unveil(defines_file_name, "cw") == -1)
+   fatal_perror("unveil");
defines_file = fopen(defines_file_name, "w");
if (defines_file == NULL)
open_write_error(defines_file_name);
@@ -330,24 +338,30 @@ open_files(void)
if (fd == -1 || (union_file = fdopen(fd, "w")) == NULL)
open_error(union_file_name);
}
+   if (unveil(output_file_name, "cw") == -1)
+   fatal_perror("unveil");
output_file = fopen(output_file_name, "w");
if (output_file == 0)
open_error(output_file_name);
 
if (rflag) {
+   if (unveil(code_file_name, "cw") == -1)
+   fatal_perror("unveil");
code_file = fopen(code_file_name, "w");
if (code_file == 0)
open_error(code_file_name);
} else
code_file = output_file;
+   if (unveil(NULL, NULL) == -1)
+   fatal_perror("unveil");
 }
 
 
 int
 main(int argc, char *argv[])
 {
-   if (pledge("stdio rpath wpath cpath", NULL) == -1)
-   fatal("pledge: invalid arguments");
+   if (pledge("stdio rpath wpath cpath unveil", NULL) == -1)
+   fatal_perror("pledge");
 
set_signals();
getargs(argc, argv);



Re: yacc + unveil

2018-09-24 Thread Theo de Raadt
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

Michael Mikonos  wrote:

> Hello,
> 
> I haven't tried using unveil() before but yacc cleanly annotates
> all the files it needs in open_files(). The options -d -r -v each
> cause an extra file to be written. unveil() is only needed for
> the input file if not reading from stdin. Temporary files are
> always under /tmp because TMPDIR environment variable was previously
> removed. OK, or any suggestions?
> 
> - Michael
> 
> 
> 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.c25 May 2017 20:11:03 -  1.29
> +++ main.c25 Sep 2018 03:43:23 -
> @@ -305,10 +305,12 @@ open_files(void)
>   create_file_names();
>  
>   if (input_file == 0) {
> + unveil(input_file_name, "r");
>   input_file = fopen(input_file_name, "r");
>   if (input_file == 0)
>   open_error(input_file_name);
>   }
> + unveil("/tmp", "crw");
>   fd = mkstemp(action_file_name);
>   if (fd == -1 || (action_file = fdopen(fd, "w")) == NULL)
>   open_error(action_file_name);
> @@ -318,11 +320,13 @@ open_files(void)
>   open_error(text_file_name);
>  
>   if (vflag) {
> + unveil(verbose_file_name, "cw");
>   verbose_file = fopen(verbose_file_name, "w");
>   if (verbose_file == 0)
>   open_error(verbose_file_name);
>   }
>   if (dflag) {
> + unveil(defines_file_name, "cw");
>   defines_file = fopen(defines_file_name, "w");
>   if (defines_file == NULL)
>   open_write_error(defines_file_name);
> @@ -330,23 +334,26 @@ open_files(void)
>   if (fd == -1 || (union_file = fdopen(fd, "w")) == NULL)
>   open_error(union_file_name);
>   }
> + unveil(output_file_name, "cw");
>   output_file = fopen(output_file_name, "w");
>   if (output_file == 0)
>   open_error(output_file_name);
>  
>   if (rflag) {
> + unveil(code_file_name, "cw");
>   code_file = fopen(code_file_name, "w");
>   if (code_file == 0)
>   open_error(code_file_name);
>   } else
>   code_file = output_file;
> + unveil(NULL, NULL);
>  }
>  
>  
>  int
>  main(int argc, char *argv[])
>  {
> - if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + if (pledge("stdio rpath wpath cpath unveil", NULL) == -1)
>   fatal("pledge: invalid arguments");
>  
>   set_signals();
> 



yacc + unveil

2018-09-24 Thread Michael Mikonos
Hello,

I haven't tried using unveil() before but yacc cleanly annotates
all the files it needs in open_files(). The options -d -r -v each
cause an extra file to be written. unveil() is only needed for
the input file if not reading from stdin. Temporary files are
always under /tmp because TMPDIR environment variable was previously
removed. OK, or any suggestions?

- Michael


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 -  1.29
+++ main.c  25 Sep 2018 03:43:23 -
@@ -305,10 +305,12 @@ open_files(void)
create_file_names();
 
if (input_file == 0) {
+   unveil(input_file_name, "r");
input_file = fopen(input_file_name, "r");
if (input_file == 0)
open_error(input_file_name);
}
+   unveil("/tmp", "crw");
fd = mkstemp(action_file_name);
if (fd == -1 || (action_file = fdopen(fd, "w")) == NULL)
open_error(action_file_name);
@@ -318,11 +320,13 @@ open_files(void)
open_error(text_file_name);
 
if (vflag) {
+   unveil(verbose_file_name, "cw");
verbose_file = fopen(verbose_file_name, "w");
if (verbose_file == 0)
open_error(verbose_file_name);
}
if (dflag) {
+   unveil(defines_file_name, "cw");
defines_file = fopen(defines_file_name, "w");
if (defines_file == NULL)
open_write_error(defines_file_name);
@@ -330,23 +334,26 @@ open_files(void)
if (fd == -1 || (union_file = fdopen(fd, "w")) == NULL)
open_error(union_file_name);
}
+   unveil(output_file_name, "cw");
output_file = fopen(output_file_name, "w");
if (output_file == 0)
open_error(output_file_name);
 
if (rflag) {
+   unveil(code_file_name, "cw");
code_file = fopen(code_file_name, "w");
if (code_file == 0)
open_error(code_file_name);
} else
code_file = output_file;
+   unveil(NULL, NULL);
 }
 
 
 int
 main(int argc, char *argv[])
 {
-   if (pledge("stdio rpath wpath cpath", NULL) == -1)
+   if (pledge("stdio rpath wpath cpath unveil", NULL) == -1)
fatal("pledge: invalid arguments");
 
set_signals();