Originally posted by [b]theultramage[/b]
http://www.eathena.ws/board/index.php?autocom=bugtracker&showbug=2939
There is a nasty defect in clif_parse_WantToConnection(), present since
r12223, which allows any unauthenticated tcp connection to disconnect arbitrary online players. The only information needed is the "account id", which can be easily obtained ingame just by coming in range of the victim. The only indication that this exploit has been used is a popup on the victim's client saying "another user has logged in with this ID", and perhaps a harmless looking connection request in the mapserver console log.
This issue was fixed in
r13656 for both trunk and stable. Server owners are advised to to update (or at least apply this fix) as soon as possible to prevent further abuse.
Following are the implementation details:
CODE
void clif_parse_WantToConnection(int fd, TBL_PC* sd)
{
...
account_id = RFIFOL(fd, packet_db[packet_ver][cmd].pos[0]);
...
bl = map_id2bl(account_id);
...
if (bl ||
((node=chrif_search(account_id)) && //An already existing node is valid only if it is for this login.
!(node->account_id == account_id && node->char_id == char_id && node->state == ST_LOGIN)))
{
sd = BL_CAST(BL_PC, bl);
if (!sd)
; //We have another char with the same account logging in/out.
else //Already connected player.
if (sd->fd)
clif_authfail_fd(sd->fd, 2); //someone else logged in
else
if(sd->state.autotrade)
map_quit(sd);// kick autotrading character
//Else do not kick character, it could be on its 10 sec penalty for Alt+F4
clif_authfail_fd(fd, 8); //Still recognizes last connection
return;
}
Here you can see that if the player is already online, 'bl' will be not-null, will match that "if( bl )", and enter the block of code that will ultimately end with clif_authfail_fd(sd->fd, 2); //someone else logged in.
This execution path bypasses all checks that verify if the attacker's request is legitimate. So this is basically another "trusting the client" bug. No request to kick a player should be accepted without first confirming that the issuer of the request has the necessary authorization - which means logging in with the correct username and password and getting the charserver to confirm the issuer's login_id1 and login_id2 values.
But what happens here is that this code runs at the very beginning, long before any authentication is requested from the charserver via chrif_authreq(). This can't be fixed properly without rewriting the auth system.
To hotpatch this, I'll assume that during normal operation, all dual-login kick requests have to go through the charserver, and the charserver is the one that issues the kick request. Therefore the above code should never execute with a properly setup auth system. Removing it will make chars unkickable if online_check is turned off on the charserver, but this is the best I can do so far...
This post has been edited by theultramage: Apr 3 2009, 01:19 PM