On Sat, May 07, 2016 at 02:01:34PM -0600, Theo de Raadt wrote: > > > So I don't understand why pledge is being added here. > > > > Because you suggested it back in november... > > Of course I suggested it: I believe I said it would be nice if > the simplest of X programs were investigated. > > But everywhere else pledge was inserted, that requires (a) full > understanding of what is being done, and (b) full testing. > > It is not like I said "pledge xclock, noone gives a fuck if it > works". > > > > The addition of > > > pledge is breaking xclock. A pledge addition should not break a > > > program (coredump) in any circumstance, it should only serve to secure > > > it in conditions of true & undetected misbehaviour. Sure, pledge can > > > make programs safer, but only if they also keep working!! Otherwise > > > this is very much throwing the baby out with the bathwater. > > > > I see only one solution here: remove the pledge until Xlib can be > > changed to read XErrorDB early and then use the in-memory version for > > error reporting. > > > > ok? >
This patch forces xclock to read XErrorDB before pledge(). Further
calls to any of the X error handler will used the in-memory copy (see
libX11/src/ErrDes.c:147).
I'm not yet 100% sure if there are other code path in lib X11/libXt
that could cause an X application to read files on some events or not.
Index: xclock.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/app/xclock/xclock.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 xclock.c
--- xclock.c 11 Nov 2015 21:12:19 -0000 1.6
+++ xclock.c 8 May 2016 10:37:48 -0000
@@ -228,6 +228,14 @@ main(int argc, char *argv[])
#endif
#ifdef HAVE_PLEDGE
+ {
+ /* force reading of XErrorDB into memory to avoid adding "rpath" to
+ pledge below */
+ int r;
+ char buf[1];
+
+ r = XGetErrorDatabaseText(XtDisplay(toplevel), "XProtoError", "0", "",
buf, 1);
+ }
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
#endif
--
Matthieu Herrb
signature.asc
Description: PGP signature
