FS#4771 - Prevent admin port authentication bypass

Attached to Project: OpenTTD
Opened by Matt D. (monoid) - Wednesday, 14 September 2011, 11:18 GMT
Last edited by Remko Bijker (Rubidium) - Thursday, 15 September 2011, 18:28 GMT
Type Bug
Category Network → Admin
Status Closed
Assigned To No-one
Operating System All
Severity High
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Currently, an admin socket's state is changed to ADMIN_STATUS_ACTIVE (ie., has been authenticated) in ServerNetworkAdminSocketHandler::SendWelcome. However, this function is not only called during the handling of a ADMIN_JOIN packet by ServerNetworkAdminSocketHandler::Receive_ADMIN_JOIN, but also by ServerNetworkAdminSocketHandler::WelcomeAll, which in turn is called by NetworkServerStart() in dedicated server mode during a game (re)start.

Hence, all a user needs to do to have their admin socket marked 'ACTIVE' without needing to go through the normal authentication routine (sending a ADMIN_JOIN packet with the admin password) is to hold onto the connection on the admin port until the dedicated server restarts to a new game. At that point the connection will be incorrectly marked as ACTIVE, and they can issue i.e., rcon commands as if authenticated normally.

This patch fixes this issue. It does so by making it so the setting of an admin socket's status to ACTIVE is only done directly after checking the ADMIN_JOIN packet, and not in welcome packet sending.

It also fixes a closely related secondary issue, where certain admin packets are sent to admin connections that are not actually marked ACTIVE. It fixes this by adding a new macro to enumerate over active admin sockets only, FOR_ALL_ACTIVE_ADMIN_SOCKETS, and using it where appropriate.

The attached python script demonstrates these two issues. Start a dedicated server, and run the script (giving the ip (+ port) on the command line). Restart the dedicated server (i.e. using the 'restart' console command), and the script will show inappropriately sent packets being received, following by an execution of the 'pwd' console command via rcon after its admin connection has been incorrectly marked ACTIVE following the SERVER_WELCOME packet.
This task depends upon

Closed by  Remko Bijker (Rubidium)
Thursday, 15 September 2011, 18:28 GMT
Reason for closing:  Fixed
Additional comments about closing:  In r22934