Patchwork [1/3] contrib: add safe_getenv() support to spd_readdir

login
register
mail settings
Submitter Theodore Ts'o
Date Jan. 22, 2013, 12:09 a.m.
Message ID <1358813390-26466-1-git-send-email-tytso@mit.edu>
Download mbox | patch
Permalink /patch/214295/
State Accepted
Headers show

Comments

Theodore Ts'o - Jan. 22, 2013, 12:09 a.m.
This is part of a series of improvements from a 2008 version of
spd_readdir.c that somehow didn't make it into the version which we
checked into e2fsprogs git tree.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 contrib/spd_readdir.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)
Florian Weimer - Jan. 23, 2013, 12:17 p.m.
* Theodore Ts'o:

> +#if HAVE___SECURE_GETENV
> +	return __secure_getenv(arg);
> +#else
> +	return getenv(arg);
> +#endif

glibc 2.17 has secure_getenv, but not __secure_getenv.  Unfortuantely,
this was the only way to turn this into an official interface.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - Jan. 23, 2013, 4:22 p.m.
On Wed, Jan 23, 2013 at 01:17:25PM +0100, Florian Weimer wrote:
> 
> glibc 2.17 has secure_getenv, but not __secure_getenv.  Unfortuantely,
> this was the only way to turn this into an official interface.

Thanks for pointing this out.  I'm using Debian testing which is still
using glibc 2.13.  The bigger issue is that it's not just
spd_readdir.c (which is in contrib and so it's not compiled from the
makefile), but we are using __secure_getenv() in libext2fs and other
libraries in e2fprogs.

Use of __secure_getenv() is protected with a configure.in test, so we
won't break when we compile under glibc 2.17, but we won't have the
full benefit of using secure_getenv(), either.  We use a similar
safe_getenv() code which will skip the getenv if the process is
running setuid, or PR_GET_DUMPABLE returns 0, so hopefully that
prevents us against the worst of the security exposure, but as [1]
states, "such emulation is necessarily brittle".

[1] http://sourceware.org/glibc/wiki/Tips_and_Tricks/secure_getenv

Can someone send me a patch, please?   Or I'll put it on my todo list....

    	    	      	     	       	       - Ted

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/contrib/spd_readdir.c b/contrib/spd_readdir.c
index f89832c..30c01b3 100644
--- a/contrib/spd_readdir.c
+++ b/contrib/spd_readdir.c
@@ -27,6 +27,10 @@ 
 #define MAX_DIRSIZE	0
 
 #define DEBUG
+/* Util we autoconfiscate spd_readdir... */
+#define HAVE___SECURE_GETENV	1
+#define HAVE_PRCTL		1
+#define HAVE_SYS_PRCTL_H	1
 
 #ifdef DEBUG
 #define DEBUG_DIR(x)	{if (do_debug) { x; }}
@@ -46,6 +50,11 @@ 
 #include <dirent.h>
 #include <errno.h>
 #include <dlfcn.h>
+#ifdef HAVE_SYS_PRCTL_H
+#include <sys/prctl.h>
+#else
+#define PR_GET_DUMPABLE 3
+#endif
 
 struct dirent_s {
 	unsigned long long d_ino;
@@ -83,6 +92,27 @@  static int num_open = 0;
 static int do_debug = 0;
 #endif
 
+static char *safe_getenv(const char *arg)
+{
+	if ((getuid() != geteuid()) || (getgid() != getegid()))
+		return NULL;
+#if HAVE_PRCTL
+	if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+		return NULL;
+#else
+#if (defined(linux) && defined(SYS_prctl))
+	if (syscall(SYS_prctl, PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+		return NULL;
+#endif
+#endif
+
+#if HAVE___SECURE_GETENV
+	return __secure_getenv(arg);
+#else
+	return getenv(arg);
+#endif
+}
+
 static void setup_ptr()
 {
 	char *cp;
@@ -97,11 +127,11 @@  static void setup_ptr()
 	real_telldir = dlsym(RTLD_NEXT, "telldir");
 	real_seekdir = dlsym(RTLD_NEXT, "seekdir");
 	real_dirfd = dlsym(RTLD_NEXT, "dirfd");
-	if ((cp = getenv("SPD_READDIR_MAX_SIZE")) != NULL) {
+	if ((cp = safe_getenv("SPD_READDIR_MAX_SIZE")) != NULL) {
 		max_dirsize = atol(cp);
 	}
 #ifdef DEBUG
-	if (getenv("SPD_READDIR_DEBUG")) {
+	if (safe_getenv("SPD_READDIR_DEBUG")) {
 		printf("initialized!\n");
 		do_debug++;
 	}