On Fri, Jan 29, 2016 at 04:37:13AM +0100, Theo Buehler wrote:
> > > is it possible to get rid of proc exec? I didn't add them on my end...
>
> there are shell escapes, so they are probably needed. I don't really
> use lynx myself, but it seems to me that it's worth investigating
> tighter pledges conditionally on various "lynx -restriction=..."
> options (hopefully those can't be changed at runtime).
>
> > Also should it call "err" or "exit_immediately" on failure?
>
> I agree that the latter looks like the right way to go.
Here is a new diff for testing, with more restrictive promises.
It builds on patches and suggestions provided off-list by tb@ and
daniel@. Thanks guys for all the feedback and ideas.
The idea is to avoid using otherwise required 'getpw', 'proc', 'exec'
promises entirely. We achieve this by disabling a couple of features,
mostly removing obsolete stuff. While we are at it, we attempt to pave
the way to be able to remove even more promises in the future, and
reduce potential attack vectors.
We disable them either at compile time :
--disable-bibp-urls
--disable-dired
--disable-finger
Or by hardcoding boolean values to disable the features just before
calling pledge and entering main program loop :
no_exec = TRUE;
no_mail = TRUE;
no_shell = TRUE;
rlogin_ok = FALSE;
telnet_ok = FALSE;
Manpage has been updated to mention those restrictions.
Also, CFLAGS="-DNOUSERS" was added in the Makefile to disable getpwnam
and getpwuid.
Comments?
Index: Makefile
===================================================================
RCS file: /cvs/ports/www/lynx/Makefile,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 Makefile
--- Makefile 14 Jan 2016 10:45:07 -0000 1.22
+++ Makefile 3 Feb 2016 23:17:38 -0000
@@ -5,7 +5,7 @@ PL = 8
COMMENT = text web browser
DISTNAME = lynx${V}dev.${PL}
PKGNAME = lynx-${V}pl${PL}
-REVISION = 1
+REVISION = 2
EXTRACT_SUFX = .tar.bz2
CATEGORIES = www net
@@ -16,6 +16,7 @@ MAINTAINER = Frederic Cambus <fred@statd
# GPLv2 only
PERMIT_PACKAGE_CDROM = Yes
+# uses pledge()
WANTLIB += c crypto ncurses ssl z
MASTER_SITES = http://invisible-mirror.net/archives/lynx/tarballs/
@@ -24,11 +25,16 @@ CONFIGURE_STYLE = gnu
CONFIGURE_ARGS = --datarootdir="${PREFIX}/share/doc/lynx" \
--disable-idna \
--disable-nls \
+ --disable-bibp-urls \
+ --disable-dired \
+ --disable-finger \
--enable-default-colors \
--enable-ipv6 \
--enable-widec \
--with-ssl=/usr \
--with-zlib
+
+CONFIGURE_ENV = CFLAGS="-DNOUSERS"
MAKE_FILE = makefile
Index: patches/patch-lynx_man
===================================================================
RCS file: patches/patch-lynx_man
diff -N patches/patch-lynx_man
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-lynx_man 3 Feb 2016 23:17:38 -0000
@@ -0,0 +1,28 @@
+$OpenBSD$
+--- lynx.man.orig Thu Oct 8 02:19:45 2015
++++ lynx.man Wed Feb 3 20:21:11 2016
+@@ -591,8 +591,24 @@ flushes the cache on a proxy server
+ .TP
+ .B \-restrictions\fR=\fI[option][,option][,option]...
+ allows a list of services to be disabled selectively.
++.TP
+ Dashes and underscores in option names can be intermixed.
+ The following list is printed if no options are specified.
++.IP
++On OpenBSD the following restrictions are always enabled:
++\fBexec\fR,
++\fBmail\fR,
++and
++\fBshell\fR.
++Additionally,
++\fBbibp-urls\fR,
++\fBdired\fR,
++\fBfinger\fR,
++\fBrlogin\fR,
++and
++\fBtelnet \fR
++features have been disabled entirely.
++.IP
+ .RS
+ .TP 3
+ .B all
Index: patches/patch-src_LYMain_c
===================================================================
RCS file: patches/patch-src_LYMain_c
diff -N patches/patch-src_LYMain_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_LYMain_c 3 Feb 2016 23:17:38 -0000
@@ -0,0 +1,25 @@
+$OpenBSD$
+--- src/LYMain.c.orig Fri Dec 18 01:34:45 2015
++++ src/LYMain.c Wed Feb 3 19:50:41 2016
+@@ -2142,6 +2142,21 @@ int main(int argc,
+ }
+
+ /*
++ * Disabling features requiring 'proc' + 'exec' and calling pledge
++ */
++ no_exec = TRUE;
++ no_mail = TRUE;
++ no_shell = TRUE;
++
++ rlogin_ok = FALSE;
++ telnet_ok = FALSE;
++
++ if (pledge("stdio rpath wpath cpath fattr dns inet tty", NULL) == -1) {
++ fprintf(stderr, "%s: pledge: %s\n", getprogname(), strerror(errno));
++ exit_immediately(EXIT_FAILURE);
++ }
++
++ /*
+ * Here's where we do all the work.
+ */
+ if (dump_output_immediately) {
Index: patches/patch-src_LYUtils_c
===================================================================
RCS file: patches/patch-src_LYUtils_c
diff -N patches/patch-src_LYUtils_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_LYUtils_c 3 Feb 2016 23:17:38 -0000
@@ -0,0 +1,18 @@
+$OpenBSD$
+--- src/LYUtils.c.orig Sun Mar 22 16:38:23 2015
++++ src/LYUtils.c Sun Jan 31 07:49:03 2016
+@@ -5253,10 +5253,11 @@ const char *Home_Dir(void)
+ /*
+ * One could use getlogin() and getpwnam() here instead.
+ */
+- struct passwd *pw = getpwuid(geteuid());
++ char *home;
+
+- if (pw && pw->pw_dir) {
+- StrAllocCopy(HomeDir, pw->pw_dir);
++ home = getenv("HOME");
++ if (home && *home) {
++ StrAllocCopy(HomeDir, home);
+ } else
+ #endif
+ {
