On Sat, Jun 10, 2017 at 12:45:01AM +0200, Theo Buehler wrote:
> On Fri, Jun 09, 2017 at 11:59:44PM +0200, Theo Buehler wrote:
> > On Fri, Jun 09, 2017 at 11:55:26PM +0200, Adam Wolk wrote:
> > > On Fri, Jun 09, 2017 at 11:54:03PM +0200, Adam Wolk wrote:
> > > > On Fri, Jun 09, 2017 at 09:28:29PM +0000, [email protected] wrote:
> > > > > Hello!
> > > > > 
> > > > > Here is a patch with a pledge bugfix in netcat and some minor style
> > > > > improvements.
> > > > > 
> > > > > An example of how to trigger the bug:
> > > > > 
> > > > > $ nc -Ptest -v -c blog.tintagel.pl 443
> > > > > nc: pledge: Operation not permitted
> > > > > 
> > > > > credits to
> > > > > * awolk@ for drawing attention to netcat.
> > > > > * Juuso Lapinlampi for suggesting to alphabetically order the 
> > > > > #includes.
> > > > > * rajak for pointing out the missing space in the error message.
> > > > > * brynet for pledge style improvements.
> > > > > 
> > > > > 
> > > > 
> > > > OK awolk@ for the updated diff (I'm attaching it inline).
> > > 
> > > forgot the diff
> > 
> > I'm ok with the diff, although I really wish there was a way to simplify
> > the convoluted mess that is the pledge logic in this program.
> > 
> > How many codepaths are there in which the second group of pledge calls
> > actually does anything? Are there any?
> > 
> 
> The first group of pledges is this:
> 
>       if (family == AF_UNIX) { // if (usetls) -> bail out on line 393
>               if (pledge("stdio rpath wpath cpath tmppath unix", NULL) == -1)
>                       err(1, "pledge");
>       } else if (Fflag) { // if (usetls) -> bail out on line 397
>               if (Pflag) {
>                       if (pledge("stdio inet dns sendfd tty", NULL) == -1)
>                               err(1, "pledge");
>               } else if (pledge("stdio inet dns sendfd", NULL) == -1)
>                       err(1, "pledge");
>       } else if (Pflag) {
>               if (pledge("stdio inet dns tty", NULL) == -1)
>                       err(1, "pledge");
>       } else if (usetls) {
>               if (pledge("stdio rpath inet dns", NULL) == -1)
>                       err(1, "pledge");
>       } else if (pledge("stdio inet dns", NULL) == -1)
>               err(1, "pledge");
> 
> So we can only reach the second group of pledges if usetls is set.
> Thus, I think the following diff would be better:
> 

OK with me as discussed on IRC.

Reply via email to