xorl %eax, %eax

CVE-2008-3229: OP Local Buffer Overflow

leave a comment »

Credit: Nico Golde
Affects: OP 1.32

On 12 July 2008, N. Golde reported a local buffer overflow on OP appliction. The op utility, is a similar to sudo(8) application but it can be configured to provide non-full root privileges to common users. It was initially written by T. Christiansen and D. Koblas and now it is maintained by H. Owen.
So… enough with the crap. Where is the bug?
It is hidden inside main.c (yeap, that was irony.)
Alright.. LULZ time. Go!

778     int Go(cmd, num, argc, argv)
779     cmd_t   *cmd;
780     int     argc;
781     int     num;
782     char    **argv;
783     {
                  ...
797     #ifdef XAUTH
798             if (getenv("DISPLAY") != NULL && (cp = FindOpt(cmd, "xauth")) != NULL) {
799             struct passwd *currentpw;
800             char tmpxauth[MAXSTRLEN], *xauth, *cxauth, *display;
                 ...
816                     xauth = (char*)malloc(strlen(pw->pw_dir) + strlen("/.Xauthority") + 1);
                 ...
821                     /* Now that we know the target user, we can copy the xauth cookies */
822                     if (getenv("XAUTHORITY") != NULL) {
823                             strcpy(cxauth, getenv("XAUTHORITY"));

ROFL!!! This is like… Excuse me, 80s called! They want their bugs back!
For those of you that didn’t get it:
1) At line 816 a buffer of size of or pwd + “./Xauthority” is being allocated on the heap.
(and it is not even checked if it succeeded or it return NULL…)
2) Line 822, gets whatever the XAUTHORITY environment variable has.
3) If it contains something it copies it using strcpy() with NO bound checks being performed earlier!
Yeap, that’s the whole story about this bug.

Oh… By the way, if you are interested, this amazing application has a lot of more. Just have a look at my comment on step 1 and you’ll get an idea. Even their patch was buggy, have a look:

    -             strcpy(cxauth, getenv("XAUTHORITY")); 
    +             cxauth = strdup(getenv("XAUTHORITY")); 

That was a part of the patch, but did these guys even read the man page of strdup(3). It clearly says:

DESCRIPTION
       The  strdup()  function  returns  a  pointer to a new string which is a
       duplicate of the string s.  Memory for the new string is obtained  with
       malloc(3), and can be freed with free(3).

Yeap, they made another instance of their elite XAUTHORITY but forgot to free() it later on. I just love bug-patches! This is a simple memory leak but those guys don’t even check the return value of malloc(3)!!! This is like amazingly incredible bug in comparison to the existing ones in OP.

If you cannot trigger this vulnerability then please.. stop reading this site. Thank you in advance.
Personally, I rate this application as the most buggy program I’ve seen during 2008. And for conclusion, just some lyrics from Children Of The Grave by Black Sabbath:

Revolution in their minds the children start to march
against the world in which they have to live and all the hate thats in their hearts

I’m not sure it has something to do with this bug, but while I was writing this these lyrics where in my mind for some reason.

Written by xorl

January 4, 2009 at 00:48

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s