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:
Index: netcat.c
===================================================================
RCS file: /var/cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.183
diff -u -p -r1.183 netcat.c
--- netcat.c 26 May 2017 16:05:35 -0000 1.183
+++ netcat.c 9 Jun 2017 22:36:28 -0000
@@ -355,6 +355,9 @@ main(int argc, char *argv[])
err(1, "pledge");
} else if (pledge("stdio inet dns sendfd", NULL) == -1)
err(1, "pledge");
+ } else if (Pflag && usetls) {
+ if (pledge("stdio rpath inet dns tty", NULL) == -1)
+ err(1, "pledge");
} else if (Pflag) {
if (pledge("stdio inet dns tty", NULL) == -1)
err(1, "pledge");
@@ -478,12 +481,6 @@ main(int argc, char *argv[])
}
if (usetls) {
- if (Pflag) {
- if (pledge("stdio inet dns tty rpath", NULL) == -1)
- err(1, "pledge");
- } else if (pledge("stdio inet dns rpath", NULL) == -1)
- err(1, "pledge");
-
if (tls_init() == -1)
errx(1, "unable to initialize TLS");
if ((tls_cfg = tls_config_new()) == NULL)
@@ -510,7 +507,7 @@ main(int argc, char *argv[])
if (TLSopt & TLS_NOVERIFY) {
if (tls_expecthash != NULL)
errx(1, "-H and -T noverify may not be used"
- "together");
+ " together");
tls_config_insecure_noverifycert(tls_cfg);
}
if (TLSopt & TLS_MUSTSTAPLE)
