Hi, As seen on the openbsd-ports mailinglist it brought to my attention we should explicitly clear passwords for the slock program (memset can be optimized out by the compiler). I also went a bit further and cleared the buffers for the last character also.
Credits goes to the OpenBSD community for the reminder. Let me know what you think or if i've missed something. Patch: >From d3f47bef10384ae624e5e7f81063e1bc1f37962c Mon Sep 17 00:00:00 2001 From: Hiltjo Posthuma <[email protected]> Date: Sun, 31 Jul 2016 13:43:00 +0200 Subject: [PATCH] clear passwords with explicit_bzero Make sure to explicitly clear memory that is used for password input. memset is often optimized out by the compiler. Brought to attention by the OpenBSD community, see: https://marc.info/?t=146989502600003&r=1&w=2 Thread subject: x11/slock: clear passwords with explicit_bzero Changes: - explicit_bzero.c import from libressl-portable. - Makefile: add COMPATSRC variable for compatibility src. - config.mk: add separate *BSD section in config.mk to simply uncomment it on these platforms. --- Makefile | 6 +++--- config.mk | 4 ++++ explicit_bzero.c | 19 +++++++++++++++++++ slock.c | 8 ++++++-- util.h | 2 ++ 5 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 explicit_bzero.c create mode 100644 util.h diff --git a/Makefile b/Makefile index 86b3437..8b3e248 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ include config.mk -SRC = slock.c +SRC = slock.c ${COMPATSRC} OBJ = ${SRC:.c=.o} all: options slock @@ -35,8 +35,8 @@ clean: dist: clean @echo creating dist tarball @mkdir -p slock-${VERSION} - @cp -R LICENSE Makefile README config.def.h config.mk ${SRC} slock.1 \ - slock-${VERSION} + @cp -R LICENSE Makefile README config.def.h config.mk ${SRC} \ + explicit_bzero.c slock.1 slock-${VERSION} @tar -cf slock-${VERSION}.tar slock-${VERSION} @gzip slock-${VERSION}.tar @rm -rf slock-${VERSION} diff --git a/config.mk b/config.mk index f93879e..3afc061 100644 --- a/config.mk +++ b/config.mk @@ -18,9 +18,13 @@ LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS} LDFLAGS = -s ${LIBS} +COMPATSRC = explicit_bzero.c # On *BSD remove -DHAVE_SHADOW_H from CPPFLAGS and add -DHAVE_BSD_AUTH # On OpenBSD and Darwin remove -lcrypt from LIBS +#LIBS = -L/usr/lib -lc -L${X11LIB} -lX11 -lXext -lXrandr +#CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_BSD_AUTH -D_BSD_SOURCE +#COMPATSRC = # compiler and linker CC = cc diff --git a/explicit_bzero.c b/explicit_bzero.c new file mode 100644 index 0000000..3e33ca8 --- /dev/null +++ b/explicit_bzero.c @@ -0,0 +1,19 @@ +/* $OpenBSD: explicit_bzero.c,v 1.3 2014/06/21 02:34:26 matthew Exp $ */ +/* + * Public domain. + * Written by Matthew Dempsky. + */ + +#include <string.h> + +__attribute__((weak)) void +__explicit_bzero_hook(void *buf, size_t len) +{ +} + +void +explicit_bzero(void *buf, size_t len) +{ + memset(buf, 0, len); + __explicit_bzero_hook(buf, len); +} diff --git a/slock.c b/slock.c index c9cdee2..5fe10ba 100644 --- a/slock.c +++ b/slock.c @@ -23,6 +23,8 @@ #include <bsd_auth.h> #endif +#include "util.h" + enum { INIT, INPUT, @@ -135,7 +137,7 @@ readpw(Display *dpy, const char *pws) * timeout. */ while (running && !XNextEvent(dpy, &ev)) { if (ev.type == KeyPress) { - buf[0] = 0; + explicit_bzero(&buf, sizeof(buf)); num = XLookupString(&ev.xkey, buf, sizeof(buf), &ksym, 0); if (IsKeypadKey(ksym)) { if (ksym == XK_KP_Enter) @@ -161,14 +163,16 @@ readpw(Display *dpy, const char *pws) XBell(dpy, 100); failure = True; } + explicit_bzero(&passwd, len); len = 0; break; case XK_Escape: + explicit_bzero(&passwd, len); len = 0; break; case XK_BackSpace: if (len) - --len; + passwd[len--] = 0; break; default: if (num && !iscntrl((int)buf[0]) && (len + num < sizeof(passwd))) { diff --git a/util.h b/util.h new file mode 100644 index 0000000..6f748b8 --- /dev/null +++ b/util.h @@ -0,0 +1,2 @@ +#undef explicit_bzero +void explicit_bzero(void *, size_t); -- 2.8.4 -- Kind regards, Hiltjo
