From 055121df80a50c47efaf7e93d7f6ddff4f483da1 Mon Sep 17 00:00:00 2001 From: Patric Stout Date: Sat, 9 Jul 2022 12:28:09 +0200 Subject: [PATCH] Fix: depot-related commands did not validate depot tiles properly (#9948) The bug comes in two slices: 1) the functions never actually checked if "tile" was a depot tile. This allowed executing the function on tile 0, where are the things like shadows of aircrafts are. 2) BuildDepotVehicleList() first checked if a vehicle is in a depot before checking if it was a primary vehicle. This is invalid for aircraft. Fixing the first hides the second, and fixing the second makes the first non-exploitable. But, fixing both felt like the best thing to do. --- src/vehicle_cmd.cpp | 2 ++ src/vehiclelist.cpp | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/vehicle_cmd.cpp b/src/vehicle_cmd.cpp index 2261456a33..373a92cdbf 100644 --- a/src/vehicle_cmd.cpp +++ b/src/vehicle_cmd.cpp @@ -634,6 +634,7 @@ CommandCost CmdMassStartStopVehicle(DoCommandFlag flags, TileIndex tile, bool do if (vehicle_list_window) { if (!GenerateVehicleSortList(&list, vli)) return CMD_ERROR; } else { + if (!IsDepotTile(tile) || !IsTileOwner(tile, _current_company)) return CMD_ERROR; /* Get the list of vehicles in the depot */ BuildDepotVehicleList(vli.vtype, tile, &list, nullptr); } @@ -666,6 +667,7 @@ CommandCost CmdDepotSellAllVehicles(DoCommandFlag flags, TileIndex tile, Vehicle CommandCost cost(EXPENSES_NEW_VEHICLES); if (!IsCompanyBuildableVehicleType(vehicle_type)) return CMD_ERROR; + if (!IsDepotTile(tile) || !IsTileOwner(tile, _current_company)) return CMD_ERROR; /* Get the list of vehicles in the depot */ BuildDepotVehicleList(vehicle_type, tile, &list, &list); diff --git a/src/vehiclelist.cpp b/src/vehiclelist.cpp index 18dbef35f2..111762ee13 100644 --- a/src/vehiclelist.cpp +++ b/src/vehiclelist.cpp @@ -80,21 +80,21 @@ void BuildDepotVehicleList(VehicleType type, TileIndex tile, VehicleList *engine case VEH_TRAIN: { const Train *t = Train::From(v); if (t->IsArticulatedPart() || t->IsRearDualheaded()) continue; - if (t->track != TRACK_BIT_DEPOT) continue; + if (!t->IsInDepot()) continue; if (wagons != nullptr && t->First()->IsFreeWagon()) { if (individual_wagons || t->IsFreeWagon()) wagons->push_back(t); continue; } + if (!t->IsPrimaryVehicle()) continue; break; } default: + if (!v->IsPrimaryVehicle()) continue; if (!v->IsInDepot()) continue; break; } - if (!v->IsPrimaryVehicle()) continue; - engines->push_back(v); }