[PATCH] skalibs: don't use the global satmp in rm_rf

From: Rasmus Villemoes <rasmus.villemoes_at_prevas.dk>
Date: Tue, 26 Sep 2017 11:58:15 +0200

At least s6-rc-init contains a call

  rm_rf(satmp.s)

which is bad, because the first thing rm_rf_tmp does it to copy the
given string to the given stralloc, which means that the .s member of
that stralloc gets realloc'ed; so when we (effectively) do
rm_rf_tmp(satmp.s, &satmp), the subsequent copy is a prototypical
use-after-free.

One could whack-a-mole this and make s6-rc-init pass a separate stralloc
and use rm_rf_tmp directly, but doing this in the rm_rf helper may fix
other applications out there.

(In general, the skalibs interfaces should do it this way, and leave the
global satmp for the application to use, to avoid exactly this kind of
bug.)

Todo: doc update.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes_at_prevas.dk>

---
 src/libstddjb/rm_rf.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/libstddjb/rm_rf.c b/src/libstddjb/rm_rf.c
index 3f8c00d..b31cf97 100644
--- a/src/libstddjb/rm_rf.c
+++ b/src/libstddjb/rm_rf.c
_at_@ -1,11 +1,12 @@
 /* ISC license. */
 
-/* MT-unsafe */
-
 #include <skalibs/skamisc.h>
 #include <skalibs/djbunix.h>
 
 int rm_rf (char const *filename)
 {
-  return rm_rf_tmp(filename, &satmp) ;
+  stralloc sa = STRALLOC_ZERO ;
+  int ret = rm_rf_tmp(filename, &sa) ;
+  stralloc_free(&sa) ;
+  return ret ;
 }
-- 
2.7.4

Received on Tue Sep 26 2017 - 09:58:15 UTC