Just two comments.
> Index: patches/patch-cvsweb_cgi
> ===================================================================
> RCS file: /data/cvs/ports/devel/cvsweb/patches/patch-cvsweb_cgi,v
> retrieving revision 1.13
> diff -u -p -r1.13 patch-cvsweb_cgi
> --- patches/patch-cvsweb_cgi 7 Apr 2013 20:07:24 -0000 1.13
> +++ patches/patch-cvsweb_cgi 27 Sep 2019 07:20:20 -0000
> @@ -1,6 +1,7 @@
> $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/04/07 20:07:24 naddy Exp $
> ---- cvsweb.cgi.orig Thu Sep 26 22:56:05 2002
> -+++ cvsweb.cgi Sun Apr 7 14:15:55 2013
> +Index: cvsweb.cgi
> +--- cvsweb.cgi.orig
> ++++ cvsweb.cgi
> @@ -1,4 +1,4 @@
> -#!/usr/bin/perl -wT
> +#!/usr/bin/perl -w
any reason to remove the taint flag on perl shebang ?
> @@ -37,7 +38,43 @@ $OpenBSD: patch-cvsweb_cgi,v 1.13 2013/0
> );
>
> @LOGSORTKEYS = qw(cvs date rev);
> -@@ -2014,20 +2009,6 @@ sub doDiff($$$$$$) {
> +@@ -249,7 +244,26 @@ EOM
> +
> + use Time::Local ();
> + use IPC::Open2 qw(open2);
> ++use OpenBSD::Pledge;
> ++use OpenBSD::Unveil;
> +
> ++pledge( qw( rpath proc exec prot_exec unveil ) ) || die "Can't pledge: $!";
> ++
> ++# directories
> ++unveil( "/usr/libdata/perl5/", "r" ) || die "Unable to unveil: $!";
> ++unveil( "/cvs/", "r" ) || die "Unable to unveil: $!";
> ++unveil( "/conf/", "r" ) || die "Unable to unveil: $!";
> ++
> ++# files
> ++unveil( "/dev/null", "r" ) || die "Unable to unveil: $!";
> ++unveil( "/usr/bin/rcsdiff", "rx" ) || die "Unable to unveil: $!";
> ++unveil( "/usr/bin/rlog", "rx" ) || die "Unable to unveil: $!";
> ++unveil( "/usr/bin/cvs", "rx" ) || die "Unable to unveil: $!";
> ++unveil( "/usr/bin/uname", "rx" ) || die "Unable to unveil: $!";
> ++
> ++unveil() || die "Unvable to unveil: $!";
> ++
> ++
the proper idiom is :
- unveil() calls first
- pledge() call without "unveil"
you will have almost(*) the same result that what you have, but it is more obvious
that unveil(2) is not permitted after pledge(2) call.
(*) almost: with your diff, calling unveil(2) later result in an error (EPERM) ;
whereas without "unveil" in pledge(2) is would result a program terminaison
(pledge violation).
And if you want to avoid patching /dev/null usage, you could add "wpath" in
pledge(2) and unveil("/dev/null", "rw"). The capability to write will be
restricted to only /dev/null with pledge+unveil. so no other write will be
allowed on filesystem.
--
Sebastien Marie
No comments:
Post a Comment