About Archive Tags RSS Feed

 

Game auditing

23 April 2005 21:50

Auditing

I spent a long time yesterday downloading source code and running quick audits of them between trying to do other things.

Lots of fun, still, but nothing found.

It's frustrating sometimes seeing code which is bad but not used, because there's the knowlege that it might become live at a later date.

Here's two examples of code which isn't used:

gnome-games

When saving the highscore file for gtali the scorefile is saved to a temporary file, then renamed to be the real file.

The code that does this, in the setgid(games) binary looks like this:

#ifdef HAVE_RENAME
        if (rename(tmpfile, scorefile))
        {
                say("rename failed!");
                unlink(tmpfile);
        }
#else
        {
                char scall[300];
                sprintf(scall, "mv %s %s", tmpfile, scorefile);
                system(scall);
        }
#endif

As no privileges are dropped the unqualified "mv" command will run with privileges setgid(games) (Indeed it must to work with /var/games, which is not writable by a typical user).

So, a trojan "mv" command will easily gain gid(games).

On none of the platforms I tested this code is used, because they all have HAVE_RENAME defined.

omega-rpg

The following code comes from the "save game" section of the code again! It firstly makes sure that the state is saved properly.

If the magic symbol "COMPRESS_SAVE_FILES" is defined it will invoke a compression program upon the saved file, and delete the uncompressed file.

In Debian packages the compression code is disabled.

The program is setgid(games), and doesn't drop its privileges. However COMPRESSOR is defined as just "gzip", not "/bin/gzip". So gaining gid(games) is trivially possible, via a fake "gzip" binary early on your PATH.

#ifdef COMPRESS_SAVE_FILES
    if (writeok &;&; compress) {
      print2("Compressing Save File....");

      strcpy(temp,COMPRESSOR);
      strcat(temp," ");
      strcat(temp,savestr);
      system(temp);
      sprintf(temp, "%s.%s", savestr, COMPRESS_EXT);
      unlink(savestr);
      link(temp, savestr);
      unlink(temp);    /* renames, but sys-V doesn't have rename()... */
    }
#endif

Pro's : The code is disabled.

Con's : Possibly upstream wasn't meant to be setgid anyway?

So, two games, two potential vulnerabilities.

| No comments