On Sat, Jul 30, 2016 at 06:30:40PM +0200, Joerg Jung wrote:
>
> > Am 30.07.2016 um 18:07 schrieb Theo Buehler <t...@math.ethz.ch>:
> >
> > Currently, slock only clears the entered passwd buffer as part of
> > auth_userokay(3). If the user aborts the password entry with ESC or
> > clears the entered password with multiple backspaces, a cleartext
> > copy of the entered password is kept in memory. Use explicit_bzero()
> > to avoid this.
>
> I think this is a useful addition.
>
> You should try to bring this patch upstream (but be
> prepared for flames).
Thanks, I will try to do that when I have a patch that compiles without
additional warnings.
> slock already contains various ifdef's so maybe add
> an #ifdef __OpenBSD__ around the explicit_bzero()
> because AFAIK it is non-portable.
Right, added.
> > While there, add __dead and printf attributes to the die() function.
> >
> >
> > I've been running with this patch for quite a while (> 3 months) with no
> > issues on amd64 and macppc.
> >
> > The port runs just fine, but when I compile it, I get the following
> > warning which I don't know how to fix properly:
>
> > CFLAGS = -O2 -pipe -std=c99 -pedantic -Wall -I/usr/X11R6/include
> > -DVERSION="1.3" -DHAVE_BSD_AUTH
> > LDFLAGS = -L/usr/X11R6/lib -lX11 -lXext -lXrandr
> > CC = cc
> > creating config.h from config.def.h
> > cc -c -O2 -pipe -std=c99 -pedantic -Wall -I/usr/X11R6/include
> > -DVERSION=\"1.3\" -DHAVE_BSD_AUTH slock.c
> > In file included from /usr/X11R6/include/X11/Xlib.h:47,
> > from /usr/X11R6/include/X11/extensions/Xrender.h:28,
> > from /usr/X11R6/include/X11/extensions/Xrandr.h:33,
> > from slock.c:16:
> > /usr/X11R6/include/X11/Xfuncproto.h:159:24: warning: ISO C does not permit
> > named variadic macros
> > slock.c: In function 'readpw':
> > slock.c:175: warning: implicit declaration of function 'explicit_bzero'
> >
> > Could someone help me with fixing that, please?
>
> The explicit prototype for the function is missing,
> try adding #include <strings.h>
Thanks. That's what I first thought as well, I should have mentioned
that. The prototype of explicit_bzero is in <string.h> which is already
included by slock.c. The problem seems to be that it is guarded by
#if __BSD_VISIBLE
and I don't know why that isn't defined and what the proper Makefile
incantations (or CFLAGS?) are in this situation.
Index: Makefile
===================================================================
RCS file: /cvs/ports/x11/slock/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- Makefile 6 Jul 2016 21:34:15 -0000 1.14
+++ Makefile 30 Jul 2016 16:59:04 -0000
@@ -3,7 +3,7 @@
COMMENT= simple X screen locker
DISTNAME= slock-1.3
-REVISION= 0
+REVISION= 1
CATEGORIES= x11
Index: patches/patch-slock_c
===================================================================
RCS file: patches/patch-slock_c
diff -N patches/patch-slock_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-slock_c 30 Jul 2016 16:59:04 -0000
@@ -0,0 +1,40 @@
+$OpenBSD$
+--- slock.c.orig Fri Feb 12 20:29:02 2016
++++ slock.c Sat Jul 30 18:48:34 2016
+@@ -48,6 +48,9 @@ static int rrevbase;
+ static int rrerrbase;
+
+ static void
++#ifdef __OpenBSD__
++ __attribute__((__noreturn__, __format__(printf, 1, 2)))
++#endif
+ die(const char *errstr, ...)
+ {
+ va_list ap;
+@@ -172,10 +175,16 @@ readpw(Display *dpy, const char *pws)
+ break;
+ case XK_Escape:
+ len = 0;
++#ifdef __OpenBSD__
++ explicit_bzero(passwd, sizeof(passwd));
++#endif
+ break;
+ case XK_BackSpace:
+ if (len)
+ --len;
++#ifdef __OpenBSD__
++ explicit_bzero(passwd + len, 1);
++#endif
+ break;
+ default:
+ if (num && !iscntrl((int)buf[0]) && (len + num
< sizeof(passwd))) {
+@@ -185,6 +194,9 @@ readpw(Display *dpy, const char *pws)
+ break;
+ }
+ color = len ? INPUT : (failure || failonclear ? FAILED
: INIT);
++#ifdef __OpenBSD__
++ explicit_bzero(buf, sizeof(buf));
++#endif
+ if (running && oldc != color) {
+ for (screen = 0; screen < nscreens; screen++) {
+ XSetWindowBackground(dpy,
locks[screen]->win, locks[screen]->colors[color]);
