Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RCON kick/ban can crash the server #7976

Closed
James103 opened this issue Feb 5, 2020 · 2 comments
Closed

RCON kick/ban can crash the server #7976

James103 opened this issue Feb 5, 2020 · 2 comments
Milestone
1.10.0

Comments

@James103
Copy link
Contributor

James103 commented Feb 5, 2020 •
edited

WARNING

This exploit can be used to crash any server provided you know the rcon password and that the server has the console commands rcon, kick, and ban enabled.

Version of OpenTTD

58c8ff4

Expected result

RCON kick/ban does not crash the server. If used to target yourself (using your IP address), RCON kick/ban fails.

Actual result

RCON kick/ban crashes your server if used to target yourself by specifying your exact address.
Server crash log
(Note: Some details of the crash log, such as the location and parts of the stack trace, can vary widely between successful reproductions.)

Steps to reproduce

  1. Server: Host a game.
  2. Server: Set your RCON password to some value (n).
  3. Client: Join the game.
  4. Client: Execute rcon (n) "kick ::1", making sure to replace ::1 with your address.
  5. Server: Crashes. Crash log attached above.
  6. Client: Ejected with "You were kicked out of the game"
@LordAro LordAro added this to the 1.10.0 milestone Feb 5, 2020
@LordAro
Copy link
Member

LordAro commented Feb 5, 2020 •
edited

Pretty sure the bug here is that you can kick the host. Everything that happens after that will likely be doing some sort of invalid memory access, making the detail of the crash nondeterministic

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00000000004ffd79 in NetworkGameSocketHandler::ReceivePackets (
    this=this@entry=0x31905400)
    at C:/msys64/home/LordAro/OpenTTD/src/network/core/tcp_game.cpp:135
135             while ((p = this->ReceivePacket()) != nullptr) {
(gdb) bt
#0  0x00000000004ffd79 in NetworkGameSocketHandler::ReceivePackets (
    this=this@entry=0x31905400)
    at C:/msys64/home/LordAro/OpenTTD/src/network/core/tcp_game.cpp:135
#1  0x000000000076c021 in TCPListenHandler<ServerNetworkGameSocketHandler, (unsigned char)0, (unsigned char)1>::Receive ()
    at C:/msys64/home/LordAro/OpenTTD/src/network/core/tcp_listen.h:124
#2  0x00000000005046a5 in NetworkGameLoop ()
    at C:/msys64/home/LordAro/OpenTTD/src/network/network.cpp:852
#3  0x0000000000561915 in GameLoop ()
    at C:/msys64/home/LordAro/OpenTTD/src/openttd.cpp:1460
#4  0x00000000006989f0 in VideoDriver_Win32::MainLoop (this=<optimized out>)
    at C:/msys64/home/LordAro/OpenTTD/src/video/win32_v.cpp:1263
#5  0x000000000055f60c in openttd_main (argc=argc@entry=1,
    argv=argv@entry=0xb30fc00)
    at C:/msys64/home/LordAro/OpenTTD/src/openttd.cpp:858
#6  0x0000000000571624 in WinMain (hInstance=<optimized out>,
    hPrevInstance=hPrevInstance@entry=0x0, lpCmdLine=<optimized out>,
    nCmdShow=<optimized out>)
    at C:/msys64/home/LordAro/OpenTTD/src/os/windows/win32.cpp:441
#7  0x00000000008f8fe2 in main (flags=<optimized out>,
    cmdline=<optimized out>, inst=<optimized out>)

@LordAro
Copy link
Member

LordAro commented Feb 5, 2020

Typically enough, the server is protected from kicking itself for client id, but not IP:

OpenTTD/src/console_cmds.cpp

Lines 477 to 505 in 58c8ff4

if (strchr(argv, '.') == nullptr && strchr(argv, ':') == nullptr) { // banning with ID
ClientID client_id = (ClientID)atoi(argv);
/* Don't kill the server, or the client doing the rcon. The latter can't be kicked because
* kicking frees closes and subsequently free the connection related instances, which we
* would be reading from and writing to after returning. So we would read or write data
* from freed memory up till the segfault triggers. */
if (client_id == CLIENT_ID_SERVER || client_id == _redirect_console_to_client) {
IConsolePrintF(CC_ERROR, "ERROR: Silly boy, you can not %s yourself!", ban ? "ban" : "kick");
return true;
}
NetworkClientInfo *ci = NetworkClientInfo::GetByClientID(client_id);
if (ci == nullptr) {
IConsoleError("Invalid client");
return true;
}
if (!ban) {
/* Kick only this client, not all clients with that IP */
NetworkServerKickClient(client_id, reason);
return true;
}
/* When banning, kick+ban all clients with that IP */
n = NetworkServerKickOrBanIP(client_id, ban, reason);
} else {
n = NetworkServerKickOrBanIP(argv, ban, reason);
}

glx22 added a commit to glx22/OpenTTD that referenced this issue Feb 7, 2020
@glx22
Fix OpenTTD#7976: Don't kick the client doing the rcon
af7226f
@nielsmh nielsmh closed this as completed in 2b1a7ce Feb 8, 2020
douiwby pushed a commit to douiwby/OpenTTD that referenced this issue Apr 16, 2020
@glx22 @douiwby
Fix OpenTTD#7976: Don't kick the client doing the rcon
c04217c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.0
Development

No branches or pull requests
2 participants
@LordAro @James103