GNOME Bugzilla – Bug 702077
Use-after-free in environment due to unsafe use of putenv().
Last modified: 2013-06-12 12:30:05 UTC
==14004== Invalid read of size 8 ==14004== at 0x3909238724: getenv (getenv.c:80) ==14004== by 0x3909304AE7: __res_vinit (res_init.c:471) ==14004== by 0x39092DAA06: gaih_inet (getaddrinfo.c:806) ==14004== by 0x39092DE6FC: getaddrinfo (getaddrinfo.c:2396) ==14004== Address 0x1a133af8 is 88 bytes inside a block of size 632 free'd ==14004== at 0x4A082F7: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==14004== by 0x390923891F: __add_to_environ (setenv.c:142) ==14004== by 0x39092387CC: putenv (putenv.c:78) ==14004== by 0x4A0C4E7: putenv (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==14004== by 0x3924435B10: set_tz (icaltime.c:342) ==14004== by 0x39244367B4: icaltime_as_timet_with_zone (icaltime.c:435) ==14004== by 0x3DE1234A4B: time_from_isodate (e-cal-time-util.c:656) One thread is modifying the environment, while another thread is attempting to read from it. This leads to a use-after-free and potentially a crash. This is discussed in http://sourceware.org/bugzilla/show_bug.cgi?id=5069 where you can find the following quote in the tenth comment: > Ulrich Drepper 2007-09-28 01:42:15 UTC > Stop this nonsense. You very obviously know nothing about these issues. Any > program that uses putenv or setenv makes itself not thread safe. So stay away > from those functions. There will be no change to any code. That seems characteristically unambiguous: we should not be calling putenv().
With glibc at least, this code could use timegm().
(In reply to comment #0) > That seems characteristically unambiguous: we should not be calling putenv(). "We" are not. icaltime.c is from libical. Looks like someone has already filed this upstream, so I'm closing this bug as NOTGNOME. I don't know if there's anything locally we can do to work around it in the meantime. http://sourceforge.net/p/freeassociation/bugs/86/
Thanks for that reference. I've filed a patch (which solves it at least for glibc, which has timegm()) there. However, we do also have the same issue in our own code: camel/tests/smime/pgp-mime.c: setenv ("GNUPGHOME", "/tmp/camel-test/.gnupg", 1); camel/tests/smime/pgp.c: setenv ("GNUPGHOME", "/tmp/camel-test/.gnupg", 1); I'm not sure we set $PATH early enough in Evolution's main() function either.
Oops, just noticed the 'tests/' in those pgp-related paths. Those should be fine too. It's just if Evolution's init does manage to start any threads before setting $PATH that we have a problem of our own.
Well, the g_setenv() call is just for the Windows build. We haven't had a working Windows build for a couple years now. That's effectively dead code.