Finger 79/tcp # McDonald, Dowd and Schuh Challenge: Part 2

Dave G. | November 13th, 2006 | Filed Under: Guests

Login: mdowd                   Name: Mark Dowd
      Directory: /guests/mdowd       Shell: /bin/ash
      On since Tue Sep 26 21:55:00 CDT from ???
      No Mail.
      Plan:
      ----------------------------------
      Views expressed by guest bloggers not necessarily those held by
      Matasano Chargen.

UPDATED (11/14/06 10:33AM/Eastern): This contest is closed. We need to do some judging. I approved all of the comments and answers in the meantime.

UPDATED (11/13/06 7:12PM/Eastern): The prize remains unclaimed. Challenge #3 may be OS specific, but we know it works on at least one OS (Linux).

Obligatory book plug:

We recently finished a book called “The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities”. It’s
all about finding security vulnerabilities in software, covering design
review through implementation review across a wide variety of
technologies, including Unix, Windows, network software, and the Web. It
will be published by Addison-Wesley on November 20th, who have provided
a sample chapter.

Note: In our previous post to this blog we indicated that the book would
be available on November 15th. The reason for the date change is that
there was evidently a problem with printing, which has delayed the book
by a few days. Sorry for the inconvenience!

But enough chat, on with the challenges!

—

Here’s the first one. Assume getoriginip() is only going to be 16 bytes maximum, and that you can provide a very long string to be placed in the
banner through the varargs. Also, assume that there isn’t a format string issue.

int banner(int origin, char *fmt, ...)
    {
      char buf[1024], *ptr = buf, *origin_string;
      size_t maxsize = sizeof(buf) - 1;
      va_list ap;
      ...
      switch(origin){
        case LOCAL:
          origin_string = "Local Connection";
          break;
        case REMOTE:
          origin_string = "Remote Connection";
          break;
        default:
          origin_string = "Unknown Connection";
          break;
      }
      sprintf(ptr, "|%s| ", origin_string);
      maxsize -= strlen(ptr);
      ptr += strlen(ptr);
      ...
      sprintf(ptr, "%s: ", get_origin_ip());
      ptr += strlen(ptr);
      maxsize -= strlen(ptr);
      ...
      va_start(ap, fmt);
      vsnprintf(ptr, maxsize, fmt, ap);
      va_end(ap);

—
This next one contains some code to remove a variable from the program
environment, say to strip out LD_PRELOAD for a setuid program in ld.so.
Assume you can specify any environment you want, but this code will be
run after an execve() so it’s guaranteed to be somewhat well-formed. The
first argument, var, is a pointer to the name of the environment
variable to be stripped out:

static void
     _dl_unsetenv(const char *var, char **env)
     {
             char *ep;

             while ((ep = *env)) {
                     const char *vp = var;

                     while (*vp && *vp == *ep) {
                             vp++;
                             ep++;
                     }
                     if (*vp == '\\0' && *ep++ == '=') {
                             char **P;

                             for (P = env;; ++P)
                                     if (!(*P = *(P + 1)))
                                             break;
                     }
                     env++;
             }
     }

—

Lastly, let’s consider a program that is running with setuid root
privileges and writing to a sensitive file. The file is line-based and
contains username/password pairs of the form “username:password”. If
“password” is omitted, then that user has no password. Assume that when
the function is called, the users credentials that correspond to
“username” have already been established, and this function updates
their password. You can also assume that the file_entries list has been
filled out by reading in the information from the privileged
(non-readable) password file, and that the /data directory is not
writable by non-root users. Here’s the code:

#define TEMP_PASSWORD_FILE "/data/passwords.tmp"
    #define PASSWORD_FILE "/data/passwords"

    struct password_entry
    {
        unsigned char user[32];
        unsigned char pass[32];
        struct password_entry *next;
    };

    struct password_entry *file_entries;

    int update_password(unsigned char *username,
                                    unsigned char *password)
    {
        int i, fd, rc = -1;
        struct password_entry *ent;

        if(!username || !password)
            return -1;

        for(i = 0; password[i]; i++)
        {
            if(!isalnum(password[i]))
                return -1;
        }

        block_signals();
        umask(022);

        if((fd = open(TEMP_PASSWORD_FILE,
                    O_RDWR|O_CREAT|O_EXCL|O_NOFOLLOW,
                    S_IRUSR|S_IWUSER)) < 0)
        {
            unblock_signals();
            return -1;
        }

        for(ent = file_entries; ent; ent = ent->next)
        {
            unsigned char buffer[256];

            if(strcmp(ent->user, username) == 0)
                snprintf(buffer, sizeof(buffer), "%.32s:%.32s\\n",
                    ent->user,password);
            else
                snprintf(buffer, sizeof(buffer), "%.32s:%.32s\\n",
                    ent->user,ent->pass);

            if(write(fd, buffer, strlen(buffer)) < 0)
                goto epilog;
        }

        rc = rename(TEMP_PASSWORD_FILE, PASSWORD_FILE);

    epilog:
        close(fd);
        unlink(TEMP_PASSWORD_FILE);
        unblock_signals();

        return rc;
    }

Viewing 23 Comments

    • ^
    • v
    I just ran into this site. I do not understand those challenges. What are we supposed to do with them? In none of the three I can find a hint at what I'm supposed to figure out. Are there errors in the code samples we're supposed to find, perhaps? Or write some code using those sample functions?
    • ^
    • v
    [Meta-issue: There are some C-to-HTML coding problems. Presumably '' should be '' and %.32sn should be %.32s\n. In case the comments engine wipes what I just typed: There's a missing backslash zero and a missing backslash in front of a couple of 'n' characters.]
    • ^
    • v
    (a)
    The first sequence {decrement maxsize; increment ptr;} adjusts things correctly. The second sequence {increment ptr; decrement maxsize;} is using a wrong (always zero) strlen(ptr) to bump maxsize, so vsnprintf() is passed a too-long maxsize and the buffer can be overflowed.

    (b)
    The env pointer is always incremented past the one I just copied into. So doubling up an environment variable, e.g.:
    LD_PRELOAD=xxx
    LD_PRELOAD=yyy
    PATH=...
    will only check and wipe the first occurence but not the second -- leaving an oppprtunity to pass through a dangerous environment.

    (c)
    With no content constraints on the password argument, it may contain embedded newlines allowing it to stuff in extra newline-delimited lines of its choosing.

    Real-world example was IDENT protocol processing in sendmail 8.6.9 and earlier -- IDENT servers were able to inject arbitrary information into sendmail queue files (allowing remote command execution). I and another person independently found that one back in Feburary 1995.
    • ^
    • v
    Tempel:

    There are potential security vulnerabilities in each sample. Find them, and explain them...
    • ^
    • v
    Liudvikas:

    Good call on both... sorry about that.
    • ^
    • v
    I'm pretty much stuck on the third challenge, so for now, here is what I got for the first two:
    Challenge 1:
    After the output of get_origin_ip() is formatted and appended to the buffer, ptr is first increased by the number of bytes written (i.e. ptr now points to a null byte), and maxsize is decreased by 0. That means it is possible to write up to the maximum 16 bytes returned by getoriginip beyond the end of the buffer.

    Challenge 2:

    If one has an environment where the last two environment variables are for example "LD_PRELOAD==something" and "LD_PRELOAD=nastylib", and LD_PRELOAD is to be cleared, then the innermost loop will move the last entry "down" one environment variable, overwriting the first of the two. Then env (pointing to that entry) will be increased to fetch the next variable (which doesn't exist), so env is 0 and the while loop terminates. On second though, this should work everywhere with two consecutive variables names like above, as the loop will just skip the one it copied down.
    • ^
    • v
    OK, my first (c) section is wrong due to the isalnum check. Although I could make it right by messing with my locale. But I don't think that's what you're looking for. So back to the drawing board.
    • ^
    • v
    Challenge 3:
    Now that I've heard "locale" mentioned in conjunction with "isalnum" here and there, I found out that isalnum() is locale dependent. Thus, if an attacker could use a locale where the newline and colon characters are considered alphanumeric characters, it would be possible for him or her to inject strings like "\nuser:password" into the password variable, to either create new user accounts, or add another entry for an existing user, using a different password.

    Unfortunately, I couldn't find a way to make the libc accept my user-generated LC_CTYPE files, and I couldn't find one in the system locale path that fulfills the mentioned requirements.

    Plus, I'm out of ideas on this one. I give up. :(
    • ^
    • v
    Can I get a copy of the book?

    I expanded #2 to an openbsd0day.


    If you've got a suid that sets real uid/gid = 0 then there you go :)
    • ^
    • v
    • ^
    • v
    Regarding the first challenge, there seems to be an issue regarding:

    sprintf(ptr, "%s: ", get_origin_ip());
    ptr += strlen(ptr);
    maxsize -= strlen(ptr);

    The problem is that ptr is being incremented AND THEN maxsize is being decremented with the new length of ptr. The problem is at that point strlen(ptr) will return 0, as it's already been jumped to point to the end of the current string. This leads to more data being placed in buf than it can hold.

    The line:

    vsnprintf(ptr, maxsize, fmt, ap);

    Tries to copy all the variable length data supplied in ap into ptr. Even assuming the format string is ok, that still leaves a small amount of extra data overflowing the buf variable.

    From what I can tell this extra data is of the size of strlen(origin_string) - which is not a huge amount but enough to craft an overflow.

    There may also be something funky in relation to the varargs and the ap variable popping things off the call stack, but I haven't thought that part through yet.
    • ^
    • v
    Ok, some updates on Challenge 3 ... after I spend some time to make it accept my own fabricated locales (provided the program at some point calls setlocale(LC_CTYPE, "")) with characters 0x0a and 0x3a in the "alpha" character class, I found out that this has no effect on suid binaries ;) ... At least not on a somewhat recent CentOS and Ubuntu Linux.

    Regarding my solution for Challenge 2: the additional "=" character is a result of me playing with environment variables in the shell, and should not really be needed when passing the environment as an argument to a call to execve running the vulnerable program.
    • ^
    • v
    And another update on Challenge 3:
    With the confusion about isalnum and locales gone, I was able to focus on my original idea of race conditions and possible file truncation.

    - It might be possiple for a user to start the suid program, have it populate the file_entries list and authenticate the user. Then the user could send it a SIGSTOP, suspending its operation before the program blocks signals. The user could later continue to run the program by sending it a SIGCONT to reset the password file to its original state, effectively overwriting any chances made by other users between SIGSTOP and SIGCONT.

    - The glibc implementation of rename(old, new) even documents a possible race. Should the file indicated by "new" already exist, rename() will first try to unlink(new) and then link(old, new), leaving a window of vulnerability where "new" does not exist. Depending on how the suid program populates the file_entries list, this might lead to truncation of the password file, if another instance of the suid program were running at the same time. I'm a little unsure about this one.

    - Still on the subject of rename(), after successfully executing link(old, new), glibc's rename() will call unlink(old). This opens another window of vulnerability within the suid program where it is possible for another instance to create the TEMP_PASSWORD_FILE and populate it, with the first instance yet to call unlink(TEMP_PASSWORD_FILE).

    - As I noticed just now, the last two bugs together could lead to removal of the password file as follows:

    Instance 1: creates TEMP_PASSWORD_FILE
    Instance 1: populates TEMP_PASSWORD_FILE
    Instance 1: calls rename()
    Instance 1: rename() calls unlink(PASSWORD_FILE) and link(TEMP_PASSWORD_FILE, PASSWORD_FILE)
    Instance 1: rename() calls unlink(TEMP_PASSWORD_FILE)
    ---> TEMP_PASSWORD_FILE is gone, Instance 2 can start
    Instance 2: creates TEMP_PASSWORD_FILE
    Instance 2: populates TEMP_PASSWORD_FILE
    Instance 2: calls rename()
    Instance 2: rename() calls unlink(PASSWORD_FILE)
    ---> PASSWORD_FILE is gone, now Instance 1 gets scheduled again
    Instance 1: calls unlink(TEMP_PASSWORD_FILE)
    ---> Instance 1 removes the TEMP_PASSWORD_FILE
    Instance 2: rename() tries to link(TEMP_PASSWORD_FILE, PASSWORD_FILE)
    ---> Instance 2 fails, both TEMP_PASSWORD_FILE and PASSWORD_FILE are gone.
    • ^
    • v
    I'll pop my solutions for 1 and 2 here. I more or less gave up on the third one yesterday, and now it looks like Kasperle might have a good one.

    1. After writing the origin IP, maxsize is decremented after pointer is incremented. This is just wrong, because it results in maxsize not getting decremented. This can result in an overflow, because maxsize now exceeds the amount of space left in the buffer.

    2. If you put two of the same environment variable in a row, the algorith will result in one still being present. This is because it copies the second instance into the slot formerly containing the first one, and then advances env to point to the next slot.

    3. I was working on filling up the disk to cause the write to fail when the file is part way written, and go to epilog. It would be good if I could open the tempfile after it is closed but before it is unlinked, but permissions appear to prevent that.

    I think Kasperle's answer is a lot more plausible.
    • ^
    • v
    (c)
    One other loose end is that the strcmp() should be strncmp().
    The code as written cannot change passwords for 32-byte usernames with non-empty passwords. It's a bug and a red flag at least, but I don't see an exploit there yet.

    Regarding races, the Linux man page I'm looking at guarantees that at no time does the destination name disappear.
    • ^
    • v
    I looked at the strcmp thing too, because it doesn't look like you're guaranteed nul-termination on the strings in the structs, but the strcmp is the only time a nul terminator is assumed, it could only cause the password to fail to be written (I guess it could hypothetically crash the app), and more importantly, the attacker doesn't really control the username.
    • ^
    • v
    (1), (2): Mangoboy beat me to it.
    (3) I think the trick here is to cause the file to truncate by setting RLIMIT_FSIZE prior to exec()ing this program.
    If you set the limit just so that the last write() trips partially over the limit, on Linux it will return the amount of data written, so the function will return successfully.
    • ^
    • v
    Oops, forgot to add: the trick on Linux works, in part, because the function takes care to block the signals.
    So instead of dying because of a signal when the resource limit is exceeded, it will happily chug along.
    (Though if another write() is performed, it will return an error.)
    • ^
    • v
    I think the manual page is wrong. Have a look at http://sourceware.org/cgi-bin/cvsweb.cgi/libc/s...
    • ^
    • v
    And I was wrong again ;) the code I referenced is not used on Linux, because that offers a rename() syscall. The code in the sysdeps/posix dir of glibc seems to be just a set of fallback solutions when some functions needed for posix compatiblity are not available as syscalls on a platform where glibc is to be built.

    See also http://www.gnu.org/software/libc/manual/html_no...
    • ^
    • v
    Where shall I send the solutions? :)

    I started looking at the challenges earlier today, the
    first ones took a few minutes, the third one was a bit
    trickier but I think I got it right now. ;>
    • ^
    • v
    Ah, nevermind. Just noticed the contest is closed.
    I had this page loaded in my browser since a week
    back, when I saw someone mentioning the page on IRC,
    but hadn't had time to look at it until now. At the
    time it only said:

    UPDATED (11/13/06 7:12PM/Eastern): The prize remains unclaimed. Challenge #3 may be OS specific, but we know it works on at least one OS (Linux).

    :)

    My solutions were:

    1.

    ptr += strlen(ptr);
    maxsize -= strlen(ptr);

    Uh-oh, maxsize will be decreased by 0 here
    -> potential 16 bytes stackbof.

    2.

    for (P = env;; ++P)
    if (!(*P = *(P + 1)))
    break;
    }
    env++;

    Uh-oh, what if we use two adjacent LD_PRELOAD:s? :>
    The second one will not be cleansed.

    3.

    At first I was thinking it had to do with signals,
    since SIGSTOP can't be blocked for instance. Then I
    noticed we probably couldn't do anything fun/useful
    with that & thought about whether there is a way to
    cut write() short and get an empty password for some
    user. ulimit -f / setrlimit(RLIMIT_FSIZE, ...) turned
    out to be exactly what I needed. ;>

    While the first two challenges took me perhaps five
    minutes each, this one took me probably 30-45 minutes.
    Nice ones! Interesting challenges are always
    appreciated. :)

    --
    Best Regards,
    Joel Eriksson
    CTO Bitsec AB
    • ^
    • v
    s/since a week back/since two weeks back/. Gah, time flies. :)

Trackbacks

close Reblog this comment
Powered by Disqus · Learn more
blog comments powered by Disqus