Home | Docs | Forums | Lists | Bugs | Planet | Store | GMN | Get Gentoo!

Full Text Bug Listing

e2fsprogs 1.37 contains the following fix :

* cache.c (blkid_get_cache): Ignore the BLKID_FILE environment variable if blkid_get_cache() is called from a setuid program.

The official patch is here :
=======================================================
diff -Nru a/lib/blkid/cache.c b/lib/blkid/cache.c
--- a/lib/blkid/cache.c	2005-03-22 07:01:25 -05:00
+++ b/lib/blkid/cache.c	2005-03-22 07:01:25 -05:00
@@ -41,7 +41,7 @@
 
 	if (filename && !strlen(filename))
 		filename = 0;
-	if (!filename)
+	if (!filename && (getuid() == geteuid()))
 		filename = getenv("BLKID_FILE");
 	if (!filename)
 		filename = BLKID_CACHE_FILE;
======================================================

Solar Designer (Openwall) suggested the following Linux-only (better) alternative :

======================================================
 		filename = 0;
 	if (!filename)
- 		filename = getenv("BLKID_FILE");
+ 		filename = __secure_getenv("BLKID_FILE");
 	if (!filename)
 		filename = BLKID_CACHE_FILE;
======================================================

base-system, please advise.

__secure_getenv() exists only in glibc. It's not portable and we have had
problems with patches of his in the past in which __secure_env() crept in.

How about this?

--- lib/blkid/cache.c.orig	2005-03-22 14:48:19.000000000 -0500
+++ lib/blkid/cache.c	2005-03-22 14:53:08.000000000 -0500
@@ -41,8 +41,13 @@ int blkid_get_cache(blkid_cache *ret_cac
 
 	if (filename && !strlen(filename))
 		filename = 0;
-	if (!filename)
+
+	if (!filename && (getuid() == geteuid()))
+#ifdef __UCLIBC__
 		filename = getenv("BLKID_FILE");
+#else
+		filename = __secure_getenv("BLKID_FILE");
+#endif
 	if (!filename)
 		filename = BLKID_CACHE_FILE;
 	cache->bic_filename = blkid_strdup(filename);

Created an attachment (id=54173) [details]
e2fsprogs-1.36-r3.ebuild.diff

Created an attachment (id=54174) [details]
e2fsprogs-1.36-blkid-cache.patch

I'd think it would be better to do it the other way around, eg. explictly check
for GLIBC before using  __secure_getenv. This ensures it can compile on any
other libc as opposed to just uclibc.

We should wait for upstream a little before they chose a final patch

agriffis put the following proposed solution for this. tested on uClibc and it
compiles fine after running autoconf.
http://dev.gentoo.org/~agriffis/misc/e2fsprogs-1.36-blkid-cache.patch
I mailed <tytso@mit.edu> about it, awaiting reply.

1.37 is now in portage w/out any patch here ... i'd prefer upstream handle this
which is what Nedd is pursuing

is e2fsprogs-1.35 affected or is it just 1.36 ?  if it's just 1.36, then ia64
is the only one who had that version stable [it was marked last nite ...]

I'm pretty sure it's 1.36 only. 

I know upstream was considering pushing some autoconf stuff to enable/disable
use of __secure_getenv. Maybe better to wait for their final choice.

Anybody know the attack vector here? Like nothing in e2fsprogs is suid afaik.

solar: re: attack vector
I think /bin/mount might be an attack vector.
it's suid, and uses the function in question (blkid_get_cache).
it's got the blkid stuff so you can do 'mount LABEL=foo /bar'.

Keeping the bug open until upstream final resolution, but won't generate a GLSA
as affected versions are unsupported.

Contacted upstream to see what they plan to do exactly.

A TEST version 1.38-WIP-0509 has been released... No answer from upstream btw.

Given that I had no answer from upstream on this, I tend to consider 1.37 is
probably the final upstream fix for that.

So should we consider this one fixed ?

works for me

ia64 is now stable in 1.37-r1 since ia64 is the only arch that had 1.36 stable

Closing as SOMEWHATFIXED. Please reopen if you have a better idea.