From 90bb6320e5958513d0b70b04edf1ee718cc09e30 Mon Sep 17 00:00:00 2001 From: Tomas Dittmann Date: Wed, 12 Jul 2017 21:23:50 +0200 Subject: [PATCH] Guard against invalid sprite access crashes (#5867) ride->num_vehicles not matching ride->vehicles[] in ride_prepare_breakdown() and window_ride_maintenance_dropdown(). --- distribution/changelog.txt | 1 + src/openrct2/ride/ride.c | 16 ++++++++++++++-- src/openrct2/windows/ride.c | 5 +++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/distribution/changelog.txt b/distribution/changelog.txt index 5183f09cc3..c6598d3067 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -21,6 +21,7 @@ - Fix: [#837] Can't move windows on title screen to where the toolbar would be (original bug) - Fix: [#1705] Time Twister's Medieval entrance has incorrect scrolling (original bug) - Fix: [#3178, #5456] Paths with non-ASCII characters not handled properly on macOS. +- Fix: [#3346] Crash when extra long train breaks down at the back - Fix: [#3479] Building in pause mode creates too many floating numbers, crashing the game - Fix: [#3565] Multiplayer server crash - Fix: [#3681] Steel Twister rollercoaster always shows all track designs diff --git a/src/openrct2/ride/ride.c b/src/openrct2/ride/ride.c index 5f9894bc37..e687fd8fea 100644 --- a/src/openrct2/ride/ride.c +++ b/src/openrct2/ride/ride.c @@ -2412,6 +2412,18 @@ static sint32 ride_get_new_breakdown_problem(rct_ride *ride) return BREAKDOWN_BRAKES_FAILURE; } +static void choose_random_train_to_breakdown_safe(rct_ride * ride) +{ + ride->broken_vehicle = scenario_rand() % ride->num_vehicles; + + // Prevent crash caused by accessing SPRITE_INDEX_NULL on hacked rides. + // This should probably be cleaned up on import instead. + while (ride->vehicles[ride->broken_vehicle] == SPRITE_INDEX_NULL && ride->broken_vehicle != 0) + { + --ride->broken_vehicle; + } +} + /** * * rct2: 0x006B7348 @@ -2450,7 +2462,7 @@ void ride_prepare_breakdown(sint32 rideIndex, sint32 breakdownReason) case BREAKDOWN_DOORS_STUCK_CLOSED: case BREAKDOWN_DOORS_STUCK_OPEN: // Choose a random train and car - ride->broken_vehicle = scenario_rand() % ride->num_vehicles; + choose_random_train_to_breakdown_safe(ride); ride->broken_car = scenario_rand() % ride->num_cars_per_train; // Set flag on broken car @@ -2469,7 +2481,7 @@ void ride_prepare_breakdown(sint32 rideIndex, sint32 breakdownReason) break; case BREAKDOWN_VEHICLE_MALFUNCTION: // Choose a random train - ride->broken_vehicle = scenario_rand() % ride->num_vehicles; + choose_random_train_to_breakdown_safe(ride); ride->broken_car = 0; // Set flag on broken train, first car diff --git a/src/openrct2/windows/ride.c b/src/openrct2/windows/ride.c index 2ad262765b..1d839a74cc 100644 --- a/src/openrct2/windows/ride.c +++ b/src/openrct2/windows/ride.c @@ -3794,14 +3794,15 @@ static void window_ride_maintenance_dropdown(rct_window *w, rct_widgetindex widg break; for (sint32 i = 0; i < ride->num_vehicles; ++i) { uint16 spriteId = ride->vehicles[i]; - do { + while (spriteId != SPRITE_INDEX_NULL) { vehicle = GET_VEHICLE(spriteId); vehicle->update_flags &= ~( VEHICLE_UPDATE_FLAG_BROKEN_CAR | VEHICLE_UPDATE_FLAG_7 | VEHICLE_UPDATE_FLAG_BROKEN_TRAIN ); - } while ((spriteId = vehicle->next_vehicle_on_train) != 0xFFFF); + spriteId = vehicle->next_vehicle_on_train; + } } break; case BREAKDOWN_RESTRAINTS_STUCK_CLOSED: