[|ALPHA|Mission85]

that is due to this:

Buffer overflow exploit radio/say, possible attempt from <ipaddress here>

it then kicks said client with the talks too much message

Note: not limited to just the bind u posted btw, happens to me with some other binds

This post has been edited by Mission85: 11 May 2010 - 10:22 PM

[mitsubishi]

i just sent 6 9 in localhost in ts and nothing happened.


edit:

oh, you edited your post with the 'just 6 9' out.

edit:

yes, that big bind does crash it disconnect the client ("bind b "ut_radio 6 9 I am at $location. I have a $weapon with $bullets rounds and $clips clips.")

it appears to be a 'security feature' explicitly added to the server; it seems quite quirky, unless it's 'weirdly' on purpose.

edit:

seeing if there's anything I can do, if I understand it..

edit:

it does happen if it's written directly in console too..

This post has been edited by mitsubishi: 11 May 2010 - 10:51 PM

[mitsubishi]

Ah, Look:


// When a radio text (as in "ut_radio 1 1 text") is sent, weird things start to happen
// when the text gets to be greater than 118 in length. When the text is really large the
// server will crash. There is an in-between gray zone above 118, but I don't really want
// to go there. This is the maximum length of radio text that can be sent, taking into
// account increments due to presence of '$'.
#define MAX_RADIO_STRLEN 118

They've put it there on purpose.



i.e. your message is just too large and they've put the message/disconnect there on purpose because they think anything larger may crash the server.

['they'=Rambetter's team trying to avoid exploits]


[I don't think I should be changing that without consensus.]




edit:

oh wait,

that was probably a rush design decision to disconnect the client, the message can probably be just cut to a shorter size. [at the time of check]

This post has been edited by mitsubishi: 11 May 2010 - 11:09 PM

[|ALPHA|Mission85]

no need to adjust that, however if one didnt want the person to be kicked, they could set it to send the user a warning message and ignore whatever it was they input to cause this.

SV_DropClient(cl, "talks too much"); could be changed to SV_SendServerCommand(cl, "print \"radio + message is too long!\""); or SV_SendServerCommand(cl, "cp \"radio + message is too long!\"");

it still would print a warning message to the server console with the persons ip, but it would only send a message to the person trying to use the bug/exploit instead of just kicking anyone who uses some standard binds/chat messages

idk.. hmm..

EDIT: saw ur edit, lol, good idea, i didnt think of that

This post has been edited by Mission85: 11 May 2010 - 11:12 PM

[mitsubishi]

Yes! that's good too actually, if one doesn't do the work for the 'cutting'.

i.e. solution: [or rather tweak]

• cut the message to the max length, or,
  
• ignore the message

[mitsubishi]

OK, I changed it to

Buffer overflow exploit radio/say, possible attempt from 127.0.0.1:59422
Client Command is ignored.

It was already ignoring the command anyway but now it doesn't kick the client.

This is silent to the player.

URL: ioUrTded.x86.exe.from_Rambetter_SVN_12052010+don't_kick_easily

PS. It could be 'smarter' but I guess the repository owners may do it.

PPS. Used latest mingw-w32 compiler, again that should offer no noticeable difference and hopefully better performance.

This post has been edited by mitsubishi: 12 May 2010 - 12:02 AM

[Rambetter]

OK guys, I considered silently ignoring say/radio commands that are too long, but simply kicking the client is safer. There could be some side effects of dropping a command and not kicking the client. However, this is probably safe.

For your reference here is the code to determine when a say/radio command is too long. Taken from sv_client.c:

// The value below is how many extra characters we reserve for every instance of '$' in a
// ut_radio, say, or similar client command.  Some jump maps have very long $location's.
// On these maps, it may be possible to crash the server if a carefully-crafted
// client command is sent.  The constant below may require further tweaking.  For example,
// a text of "$location" would have a total computed length of 25, because "$location" has
// 9 characters, and we increment that by 16 for the '$'.
#define STRLEN_INCREMENT_PER_DOLLAR_VAR 16

// Don't allow more than this many dollared-strings (e.g. $location) in a client command
// such as ut_radio and say.  Keep this value low for safety, in case some things like
// $location expand to very large strings in some maps.  There is really no reason to have
// more than 6 dollar vars (such as $weapon or $location) in things you tell other people.
#define MAX_DOLLAR_VARS 6

// When a radio text (as in "ut_radio 1 1 text") is sent, weird things start to happen
// when the text gets to be greater than 118 in length.  When the text is really large the
// server will crash.  There is an in-between gray zone above 118, but I don't really want
// to go there.  This is the maximum length of radio text that can be sent, taking into
// account increments due to presence of '$'.
#define MAX_RADIO_STRLEN 118

// Don't allow more than this text length in a command such as say.  I pulled this
// value out of my ass because I don't really know exactly when problems start to happen.
// This value takes into account increments due to the presence of '$'.
#define MAX_SAY_STRLEN 256

void SV_ExecuteClientCommand( client_t *cl, const char *s, qboolean clientOK ) {
        ucmd_t  *u;
        qboolean bProcessed = qfalse;
        int     argsFromOneMaxlen;
        int     charCount;
        int     dollarCount;
        int     i;
        char    *arg;
        qboolean exploitDetected;

        Cmd_TokenizeString( s );

        // see if it is a server level command
        for (u=ucmds ; u->name ; u++) {
                if (!strcmp (Cmd_Argv(0), u->name) ) {
                        u->func( cl );
                        bProcessed = qtrue;
                        break;
                }
        }

        if (clientOK) {
                // pass unknown strings to the game
                if (!u->name && sv.state == SS_GAME) {
                        Cmd_Args_Sanitize();

                        argsFromOneMaxlen = -1;
                        if (Q_stricmp("say", Cmd_Argv(0)) == 0 ||
                                        Q_stricmp("say_team", Cmd_Argv(0)) == 0) {
                                argsFromOneMaxlen = MAX_SAY_STRLEN;
                        }
                        else if (Q_stricmp("tell", Cmd_Argv(0)) == 0) {
                                // A command will look like "tell 12 hi" or "tell foo hi".  The "12"
                                // and "foo" in the examples will be counted towards MAX_SAY_STRLEN,
                                // plus the space.
                                argsFromOneMaxlen = MAX_SAY_STRLEN;
                        }
                        else if (Q_stricmp("ut_radio", Cmd_Argv(0)) == 0) {
                                // We add 4 to this value because in a command such as
                                // "ut_radio 1 1 affirmative", the args at indices 1 and 2 each
                                // have length 1 and there is a space after them.
                                argsFromOneMaxlen = MAX_RADIO_STRLEN + 4;
                        }
                        if (argsFromOneMaxlen >= 0) {
                                exploitDetected = qfalse;
                                charCount = 0;
                                dollarCount = 0;
                                for (i = Cmd_Argc() - 1; i >= 1; i--) {
                                        arg = Cmd_Argv(i);
                                        while (*arg) {
                                                if (++charCount > argsFromOneMaxlen) {
                                                        exploitDetected = qtrue; break;
                                                }
                                                if (*arg == '$') {
                                                        if (++dollarCount > MAX_DOLLAR_VARS) {
                                                                exploitDetected = qtrue; break;
                                                        }
                                                        charCount += STRLEN_INCREMENT_PER_DOLLAR_VAR;
                                                        if (charCount > argsFromOneMaxlen) {
                                                                exploitDetected = qtrue; break;
                                                        }
                                                }
                                                arg++;
                                        }
                                        if (exploitDetected) { break; }
                                        if (i != 1) { // Cmd_ArgsFrom() will add space
                                                if (++charCount > argsFromOneMaxlen) {
                                                        exploitDetected = qtrue; break;
                                                }
                                        }
                                }
                                if (exploitDetected) {
                                        Com_Printf("Buffer overflow exploit radio/say, possible attempt from %s\n",
                                                NET_AdrToString(cl->netchan.remoteAddress));
                                        SV_DropClient(cl, "talks too much");
                                        return;
                                }
                        }

                        //////////////////////////////////////////////////////////
                        // separator for logcallvote.patch and forceautojoin.patch
                        //////////////////////////////////////////////////////////

                        VM_Call( gvm, GAME_CLIENT_COMMAND, cl - svs.clients );
                }
        }
        else if (!bProcessed)
                Com_DPrintf( "client text ignored for %s: %s\n", cl->name, Cmd_Argv(0) );
}

[mitsubishi]

oops, I already changed the binary.

But I think it was a bit harsh (to kick them), since it may happen to any legitimate player that just happened to make a big radio message.




An Idea might be to also send to the player "Your command has been ignored because [..]".

Or best, prune the message to a safe length and "Your command's message has been cut to a safe length [..]". Though that may need a boolean to print it only once and not spam it all the time they use the same bind:P

Though the whole idea of 'cutting' may be problematic since cutting alone may leave an 'incorrect' command, e.g. if quotes are required at the end. Ignoring + messaging the player might be best.

Then again some could live with the tradeoff; some consoles seem to behave like that, just pruning.

This post has been edited by mitsubishi: 12 May 2010 - 12:24 AM

[uJ|V00d00]

I can understand up to this bit.

Quote

bind b "ut_radio 6 9 I am at $location."

I could also understand using the $health, but weapon, clips, etc. Most jump servers dont allow you to shoot people, so why is their a need to spam about weapons and ammo on them??

But yes kicking them is maybe a bit to harsh. In MaJ's binary their is a togglemute function, that completely mutes a player, having that fire up a temp mute for 5-10 mins would be a better solution. If they cant spam crap to server for 5 mins they will eventually learn to not do it at all.