1 Topic by Stinky

  • Stinky
  • New member
  • Offline

Topic: Security hole in server

Someone was exploiting a security hole in the server I was playing on and making it crash and restart tonight.  The bug is here in gs_server.cpp:

                if(msg == MSG_CHANGEINFO && strcmp(name, server_clientname(client_id)) != 0)
                {
                        char msg[256];
                        sprintf(msg, "*** %s changed name to %s", server_clientname(client_id), name);
                        send_chat(-1, -1, msg);
                }

The evildoer was presumably sending handcrafted messages with names larger than 256 characters.  Looks nasty...
this should be snprintf or you should check strlen(name) first.

I'm not certain, but I'm not convinced there aren't other bugs like this lurking in the message unpacking code.  This is kind of dangerous because it could give someone shell access to any machine running a server...

2 Reply by ShootMe (edited by ShootMe 2008-03-17 08:04:17)

  • ShootMe
  • ShootMe
  • Member
  • Offline

Re: Security hole in server

actually thats not the part of the code that causes the crash

unsigned char *p = (unsigned char *)name;
while (*p)
{
    if(*p < 32)
        *p = ' ';// CRASH HERE
    p++;
}

Is where the crash occurs. for some reason the server will crash when trying to change a character to something different. Having a length over 256 isnt the main issue as I tired it with only a single carrige return and server crashed. When I update my server tommorow, which is one of the servers the guy exploited he will no longer be able to crash it, at least with this method.

my fix was this as well as fixing the possibility of messages being over 256 in length:

unsigned char *p = (unsigned char *)name;
while (*p)
{
    if(*p < 32) {
        name = "...";
        break;
    }
    p++;
}