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

Reply via email to