FS#6598 - Possible for client to be in a nonexistent company on a server; but then the client crashes.

Attached to Project: OpenTTD
Opened by James (james1101) - Friday, 04 August 2017, 05:58 GMT
Last edited by dP (_dp_) - Thursday, 17 August 2017, 21:53 GMT
Type Bug
Category Core
Status With patch
Assigned To No-one
Operating System All
Severity Medium
Priority Normal
Reported Version 1.7.1
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


To Reproduce this Crash:
Start OTTD server.
Remember it's ip address.
Start OTTD client.
Client: Connect to server.
Client: Execute 'join 255'.
-> Client joins spectators.
Client: Execute 'join N', where N is greater than 15 and less than 255.
-> Client throws: 'Error: Company does not exist.'
Client: Execute 'connect <server-ip>#N', where N is greater than 15 and less than 255.
-> Client: Connect should be successful.
Client: Open company list.
-> Client crashes. Crash files are attached.

Client version 1.7.1 on Windows 7 Ultimate; Server version 1.7.1 on Windows 7 Ultimate

Suggested Fix: Make the company id part of the connect command's argument follow the same rules as the join command's company id argument, ie must be between 0 and max_companies, or 255 (spectator).
   crash.png (211.9 KiB)
   crash.log (8.8 KiB)
(application/octet-stream)    crash.dmp (12.67 MiB)
This task depends upon

Comment by Daniele (Wolf01) - Tuesday, 15 August 2017, 13:21 GMT
I can't reproduce it on Win 10.
Tried with local server both with IPv4 and IPv6.
Tried logging in as a company 1 and spectator.
Tried connecting from titlescreen.

The command already checks for join_as > MAX_COMPANIES:
if (join_as != COMPANY_SPECTATOR) {
if (join_as > MAX_COMPANIES) return false;
Comment by dP (_dp_) - Tuesday, 15 August 2017, 20:35 GMT
Ok, I didn't get any crashes, but I think it's close enough. Thing is connect cmd sometimes leaves the game in a weird half-disconnected state. It's not very noticable at first but it's clearly not a valid state. E.g. you can build rails but can't leave company or use join command. Or you can bring up mp lobby while still being in game.
There are several ways to get in that state:
1) connect ::1#254 (or anything 16-254)
2) connect ::1#1 and cancelling password prompt (company 1 has to be passworded)
3) connect ::1:0

I think I got all such cases in my patch but can't be 100% sure.
Also rearranged console output a bit so it doesn't say "connecting ..." before failing on arg parsing.
And added check for negative company # to prevent possible bugs with it.