OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
 
Subject: Re: standard libraries (STL, GLib,...)
From: David A. Wagner (dawcs.berkeley.edu)
Date: Thu Aug 24 2000 - 00:16:24 CDT

Solar Designer <solarfalse.com> wrote:
> I've just checked, -- the fix got into the CVS a few days after your
> report in May. glibc 2.1.3 was already out by the time. The glibc
> "2.1.3" as included in RH 6.2 is based on a later CVS version (so it
> isn't really 2.1.3, even without patches included in the SRPM), but
> is older than May and thus still has the bug.
>
> I've spent an hour on this and, fortunately, the bug can't be used to
> attack the dynamic linker of a SUID/SGID binary directly. It can,
> however, affect poorly written sudo-like applications, if some aren't
> smart enough to unset LD_PRELOAD and LD_LIBRARY_PATH (with a working
> unsetenv() or, better, by creating a new environ, of course). So,
> an application bug is needed to exploit this glibc bug.

Thanks for checking up on the details.

> > ObHistoricalNote: By the way, does anyone remember the bug in telnetd
> > accepting environment variables? There was a fascinating bug explained
> > there: setenv(name,val) and unsetenv(name) do not behave as expected
> > when 'name' contains an '=' character! setenv("x=y","z") defines the
> > environment variable called "x"; unsetenv("x=y") deletes the variable
> > called "x=y". Subtle, eh?
>
> Perhaps it would be nice if setenv() refused to set a variable with
> '=' in its name, what do you think?

I mostly agree, except that one can imagine some far-fetched scenario
where a program relies on setenv() to overwrite the previous name-value
binding, which can be unfortunate.

What do you think of something like the following? Is it any better?
(And, is it too ugly or too Posix-uncompliant?)

int setenv(char *name, char *value, int replace) {
  char *p = strchr(name, '=');
  if (p) {
    if (/* something; I don't know what */) {
      syslog(LOG_USER | LOG_WARNING,
        "Possible attack attempt: setenv(`%s',`%s',%d)", name, value, replace);
    }
    if (replace) {
      /* Clear any existing bindings, just in case caller wanted us to */
      unsetenv(name);
      *p = '\0';
      unsetenv(name);
    }
    /* Return, without adding any new bindings */
    return -1;
  }
  ...
}