r1ch.net forums
* Home Help Search Login Register
r1ch.net  |  r1ch.net stuff  |  R1Q2  |  Topic: R1Q2 bug: won't connect when skin is empty
Pages: [1]
Print
Author Topic: R1Q2 bug: won't connect when skin is empty  (Read 5046 times)
foc
Guest
« on: July 18, 2005, 09:30:32 pm »

Little bug: when you have set skin "" in your config, when responding the challenge, R1Q2 (I use SDLQuake2 but I assume the same happens for r1) tells the server:

0000   00 04 e2 ce fc d8 00 c0 df 0b 7d 8b 08 00 45 00  ..........}...E.
0010   00 81 00 08 40 00 40 11 f3 76 c0 a8 02 b7 3e 2c  ....@.@..v....>,
0020   45 62 bd 3a 6d 10 00 6d 9f 85 ff ff ff ff 63 6f  Eb.:m..m......co
0030   6e 6e 65 63 74 20 33 35 20 39 30 20 31 31 35 35  nnect 35 90 1155
0040   30 34 35 33 31 31 20 22 5c 73 70 65 63 74 61 74  045311 "\spectat
0050   6f 72 5c 30 5c 67 65 6e 64 65 72 5c 6d 61 6c 65  or\0\gender\male
0060   5c 66 6f 76 5c 39 30 5c 72 61 74 65 5c 38 30 30  \fov\90\rate\800
0070   30 5c 6d 73 67 5c 31 5c 6e 61 6d 65 5c 46 69 72  0\msg\1\name\Fir
0080   65 5c 68 61 6e 64 5c 32 22 20 31 33 39 30 0a     e\hand\2" 1390.

Now most servers will refuse the connection. It seems that when I change my config to say skin male/grunt it does connect.


Took me two hours to find out.
Logged
foc
Guest
« Reply #1 on: July 18, 2005, 09:32:04 pm »

The correct answer should be:

0000   00 04 e2 ce fc d8 00 c0 df 0b 7d 8b 08 00 45 00  ..........}...E.
0010   00 8d 00 07 40 00 40 11 f3 6b c0 a8 02 b7 3e 2c  ....@.@..k....>,
0020   45 62 81 03 6d 10 00 79 4b 7c ff ff ff ff 63 6f  Eb..m..yK|....co
0030   6e 6e 65 63 74 20 33 34 20 38 35 31 20 36 33 31  nnect 34 851 631
0040   33 35 33 32 35 36 20 22 5c 6d 73 67 5c 31 5c 73  353256 "\msg\1\s
0050   70 65 63 74 61 74 6f 72 5c 30 5c 72 61 74 65 5c  pectator\0\rate\
0060   38 30 30 30 5c 66 6f 76 5c 31 31 30 5c 67 65 6e  8000\fov\110\gen
0070   64 65 72 5c 6d 61 6c 65 5c 73 6b 69 6e 5c 6d 61  der\male\skin\ma
0080   6c 65 2f 67 72 75 6e 74 5c 6e 61 6d 65 5c 46 69  le/grunt\name\Fi
0090   72 65 5c 68 61 6e 64 5c 32 22 0a                 re\hand\2".
Logged
R1CH
Administrator
Member

Posts: 2625



« Reply #2 on: July 18, 2005, 10:12:56 pm »

This has nothing to do with R1Q2, servers running Q2Admin will refuse your connection with an empty skin.
Logged
QwazyWabbit
Member

Posts: 402


« Reply #3 on: July 19, 2005, 11:56:16 am »

This smells like a client bug to me. It should not be possible to null out the skin but this error is covered by the checks in q2admin. When I do skin "" in an R1Q2 client (b5510) already connected to an R1Q2 server (b5510) with a q2admin 1.17.44 overlay the skin string is empty but the scoreboard and client show a male/grunt. I don't think the target brush and skin disappear for other users so it can't be used to cheat but the server side may want to automatically reset the string if it comes nulled from the client. This would probably be mod dependent more than server engine dependent.
Logged
R1CH
Administrator
Member

Posts: 2625



« Reply #4 on: July 19, 2005, 02:53:11 pm »

How is this a client bug? Setting a userinfo var to "" removes it from the userinfo string, always has, always will. If you do set skin "" in-game on an R1Q2 server, the sv_validate_playerskins code will notice it's an illegal skin and override it with male/grunt for everyone on the server.
Logged
QwazyWabbit
Member

Posts: 402


« Reply #5 on: July 20, 2005, 01:29:32 am »

It's a bug because it can cause undefined behaviour. That is, a null skin can get you connected or not, depending on the tolerance conditions of the server setup. Since a null skin should not have been a "legal" skin in the first place, enforcement should have been in place in the client code to prevent user input from creating that condition. I realize that client-side enforcement is no guarantee, since a hacked client is still a hacked client, but it would have been one more layer of prevention or at least a minimal protection against innocent user error.

Under what conditions are null userinfo strings sent from a client "useful"? If they are useful, then why should they cause a server to reject the client? Certainly, if a server is going to reject a connection because a skin string is empty, there should be some message to that effect so the user is not left guessing as to the true cause of the rejection. Client side code that filled an empty user argument with a predefined default value would have been preferable in this case. This makes it an "implementation defect" or "inadequately defined specification" error, a bug in anyones book. Certainly no stranger to anyone who has been reading the original Q2 code.

Proper thoughtful analysis ought to include questions like: How can a user screw this up? Are all legal states defined and are all illegal states inhibited? How can this code be exploited? (Something I know you are familiar with.) Was the user input checked and sterilized before being passed? (This one has bitten me more than once, I am not ashamed to say.)

I am reminded of a case I dealt with a long time ago. This was in a CNC machine tool. The allowable input to the CNC for the feedrate was 000.0 to 400.0 inches per minute. One decimal place allowed. The customer generated a part program and output the feedrates to two decimal places. (e.g., 29.95) The control took the input tape without errors but when it computed the math all the feeds were incorrect because the feedrate "string" was converted to an internal units representation and everything was shifted. The customer was left high and dry trying to figure out why his job wouldn't run in the machine, even though the machine never generated a programming error message. No one had thought to filter the tape input through the syntax engine that drove the error displays for the very syntax errors that were impossible to produce through the keyboard. The format specification was properly documented for all to see but the system didn't perform the error checks against that standard. Not a bug?

The solution was to point out the proper format specification to the customer so he could modify his post processor to output the part program job with the correct number of digits and to modify the CNC system so it would parse the tape input through the same pathways as keyboard input as it was loaded into the machine memory.
« Last Edit: July 20, 2005, 01:37:24 am by QwazyWabbit » Logged
R1CH
Administrator
Member

Posts: 2625



« Reply #6 on: July 20, 2005, 12:03:42 pm »

A null skin is perfectly legal. Take for example the Gloom mod - user skins aren't allowed, they are all set by the server depending what class you are. Therefore to conserve space in the userinfo string for other variables needed for eg. cheat checking, Gloom will force set your skin to "".

And I agree that Q2Admin is badly designed in that it doesn't inform the user if their skin is empty, let alone no mention of what purpose a null skin check actually serves.
Logged
Bossman
Member

Posts: 486


« Reply #7 on: July 20, 2005, 08:11:17 pm »

  Its not the Q2admin it is the Admin that set it that way...The q2admin does what you set it to do..
Logged
Bossman
Member

Posts: 486


« Reply #8 on: July 20, 2005, 08:12:47 pm »

  All they need to do is add a stuff comand in server.cfg for skin then the ones that know how can fix theres and the ones that don't well they are set...
Logged
QwazyWabbit
Member

Posts: 402


« Reply #9 on: July 20, 2005, 09:35:51 pm »

How do you write a conditional stuff command in server.cfg?
Logged
Bossman
Member

Posts: 486


« Reply #10 on: July 21, 2005, 07:15:20 am »

addstuffcmd connect set skin "female/athena" or whatever
Logged
QwazyWabbit
Member

Posts: 402


« Reply #11 on: July 22, 2005, 12:37:52 pm »

This is not conditional. This is an unconditional forced skin and does nothing to prevent the player from setting a null skin after he's connected. The object is to force a skin only when the client has nulled it. You have not provided a solution for this. A game that depends on predefined player skins will/ought to/should set the skins as you suggest and monitor the infostring for changes and reset it to the designed skins (as CTF does) and ignore any client commands to change it. Server side is the only way to enforce this, that much is clear. My point is that the original design was flawed if in fact a null skin is even a problem for the game, which I don't believe is true. Q2Admin gives the ability to reject a client based on skin criterion but appears to be flawed in the way it informs the users about what the rejection is about.  I have not checked whether this can be corrected with a message in the config file. A properly designed mod would cope with skin issues internally and Q2admin would not even need this capability.
« Last Edit: July 22, 2005, 12:40:05 pm by QwazyWabbit » Logged
Bossman
Member

Posts: 486


« Reply #12 on: July 22, 2005, 06:06:37 pm »

  A null skin would go back to male/grunt if server is runnin r1q2. Therefor since it is not a ctf server like I use, I would think it does same thing as far as null skins, so if that happens then you can do the nessary stuff to that player. I guess the complaint would be they like using male/grunt then. But you can use a line in your motd to explain no male/grunt skins. If you force a skin and they change it back to a null skin and it shows up with grunt then they are not a new player at all so gettting kicked or whatever is deserving after all that. You do what you can with what you got.
Logged
Pages: [1]
Print
r1ch.net  |  r1ch.net stuff  |  R1Q2  |  Topic: R1Q2 bug: won't connect when skin is empty
Jump to:  

Powered by SMF 1.1.19 | SMF © 2013, Simple Machines