Page 1 of 3

BUGFIX: Oversize server commands

Posted: Mon Feb 02, 2004 8:42 pm
by bani
superdug wrote:The hack involves using a bufferoverflow in the sound system. A bind for a vsay is made, this vsay howerever seems to be larger than the buffer will allow and causing an immediate disconnect from all users connected to the server.
this isnt exactly correct. its not a buffer overflow of the buffer (or the server would crash).

it's basically the client engine being retarded. :curse:

if you send an oversize trap_sendservercommand(), the server happily sends the data to the client. however the client (engine, not cgame) expects server commands to never be > 1022 characters. so the client engine truncates the received servercommand at 1022, and the client engine interprets the next character in the server command string as a network command byte.

the client then gets totally confused at this point trying to interpret the rest of the string as raw network protocol, and blows up.

here's a true fix. basically it logs oversize commands and drops them on the floor. this will stop the vsay exploit, and any similar exploits in the future.

g_syscalls.c:

Code: Select all

void trap_SendServerCommand( int clientNum, const char *text ) {
        // rain - hack - commands over 1022 chars will crash the
        // client upon receipt, so ignore them
        if( strlen( text ) > 1022 ) {
                G_LogPrintf( "trap_SendServerCommand( %d, ... ) length exceeds 1022.\n", clientNum );
                G_LogPrintf( "text [%s]\n", text );
                return;
        }
        syscall( G_SEND_SERVER_COMMAND, clientNum, text );
}

Re: BUGFIX: Oversize server commands

Posted: Mon Feb 02, 2004 9:01 pm
by Badhabit
Bani
Is this for the server or the client?
And were does this (code)config need to go.

Posted: Mon Feb 02, 2004 9:09 pm
by bani
there is only one g_syscalls.c in the SDK and only one void trap_SendServerCommand in g_syscalls.c :P

Posted: Tue Feb 03, 2004 2:22 am
by bani
ok, the limit turned out to be 1022... documentation updated to reflect that.

Posted: Tue Feb 03, 2004 7:08 am
by spoon
No, DG. Yer right... you can't stick this into a pk3. :) Basically, it's a fix for the source code, which means you need to get the wet_source package, find a compiler for Windows and Linux, and then rebuild the qagame so/DLL files.

And that's on the list of stuff bani mentioned they weren't going to cover. :)

The fix is already in etpro, and this is really only relevant if you're trying to make your own mod.

Posted: Tue Feb 03, 2004 7:09 am
by DG
oopsie i deleted and posted on SD forum instead :oops:

Posted: Tue Feb 03, 2004 7:21 am
by spoon
DG: D'oh. :)

Any rate, I was just wondering...

After grep'ing through the source this weekend, I ran across SanitizeString() in g_cmds.c Could you just stick a bound check in there with something like

Code: Select all

void SanitizeString( char *in, char* out, qboolean fToLower)
{
      int cnt = 0;
      while&#40; *in && cnt++ < 1022 &#41; &#123;
              /* same stuff */
      &#125;
      *out = 0;
&#125;
Or am I overly optimistic in assuming that they call this func every time the game accepts string input from the user? :(

Posted: Tue Feb 03, 2004 4:16 pm
by ReyalP
spoon wrote: The fix is already in etpro, and this is really only relevant if you're trying to make your own mod.
Just to clarify, it is not in 2.0.7. It is fixed in the NEXT version of ETPro.

Posted: Tue Feb 03, 2004 4:40 pm
by spoon
Hrm. What I was thinking of was in the 2.0.7 announcement:
bugfix: fix bug allowing spectators to crash the server - no, we arent going to tell you how to do it :)
So this was something else besides an overflow? :(

And the hits just keeeeeeeep on comin'.

Re: BUGFIX: Oversize server commands

Posted: Wed Feb 04, 2004 6:41 pm
by ReyalP
bani wrote: g_syscalls.c:

Code: Select all

void trap_SendServerCommand&#40; int clientNum, const char *text &#41; &#123;
        // rain - hack - commands over 1022 chars will crash the
        // client upon receipt, so ignore them
        if&#40; strlen&#40; text &#41; > 1022 &#41; &#123;
                G_LogPrintf&#40; "trap_SendServerCommand&#40; %d, ... &#41; length exceeds 1022.\n", clientNum &#41;;
                G_LogPrintf&#40; "text &#91;%s&#93;\n", text &#41;;
                return;
        &#125;
        syscall&#40; G_SEND_SERVER_COMMAND, clientNum, text &#41;;
&#125;
FWIW, G_LowPrintf will only print the first 1023 chars (includng timestamp) so the above will always be truncated, eating the \n and leaving the next log message on the same line. Not that it really matters...

Posted: Wed Feb 04, 2004 7:48 pm
by bani
you could always up the buffersize ...

Posted: Tue Jun 29, 2004 10:15 am
by Chruker
Wouldn't a length check of the string in G_LogPrintf be better, than just upping the buffer?

Posted: Tue Jun 29, 2004 10:25 am
by bani
no. and we dont up the buffer either.

Posted: Sun Sep 11, 2005 2:12 pm
by NYKiller
I dont get this How do you fix it like what are the steps you have to do?

Posted: Sun Sep 11, 2005 2:58 pm
by WeblionX
You modify the source in your mod. What's this, you're making a mod and you don't know how to edit source? What, you're not making a mod?! Then what are you doing here?