From aef80b1a82bf55345cd9bba54e7ede0874d47790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Mon, 21 Nov 2016 10:37:10 +0100 Subject: [PATCH] Validate parameters passed to game commands (#4814) --- src/peep/peep.c | 6 +++++- src/peep/peep.h | 2 +- src/peep/staff.c | 18 ++++++++++++++---- src/peep/staff.h | 2 +- src/world/balloon.c | 7 ++++++- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/peep/peep.c b/src/peep/peep.c index c6294d8c44..4f42005c54 100644 --- a/src/peep/peep.c +++ b/src/peep/peep.c @@ -1990,8 +1990,12 @@ bool peep_pickup_place(rct_peep* peep, int x, int y, int z, bool apply) return true; } -bool peep_pickup_command(int peepnum, int x, int y, int z, int action, bool apply) +bool peep_pickup_command(unsigned int peepnum, int x, int y, int z, int action, bool apply) { + if (peepnum >= MAX_SPRITES) { + log_error("Failed to pick up peep for sprite %d", peepnum); + return false; + } rct_peep* peep = GET_PEEP(peepnum); if (!peep || peep->sprite_identifier != SPRITE_IDENTIFIER_PEEP) { return false; diff --git a/src/peep/peep.h b/src/peep/peep.h index c1bd61d592..bb86b94cb1 100644 --- a/src/peep/peep.h +++ b/src/peep/peep.h @@ -655,7 +655,7 @@ int peep_has_food(rct_peep* peep); void peep_pickup(rct_peep* peep); void peep_pickup_abort(rct_peep* peep, int old_x); bool peep_pickup_place(rct_peep* peep, int x, int y, int z, bool apply); -bool peep_pickup_command(int peepnum, int x, int y, int z, int action, bool apply); +bool peep_pickup_command(unsigned int peepnum, int x, int y, int z, int action, bool apply); void game_command_pickup_guest(int* eax, int* ebx, int* ecx, int* edx, int* esi, int* edi, int* ebp); void peep_sprite_remove(rct_peep* peep); void peep_remove(rct_peep* peep); diff --git a/src/peep/staff.c b/src/peep/staff.c index 0c429ac267..d8eac92e62 100644 --- a/src/peep/staff.c +++ b/src/peep/staff.c @@ -65,7 +65,12 @@ void game_command_update_staff_colour(int *eax, int *ebx, int *ecx, int *edx, in colour = (*edx >> 8) & 0xFF; if (*ebx & GAME_COMMAND_FLAG_APPLY) { - staff_set_colour(staffType, colour); + // Client may send invalid data + bool ok = staff_set_colour(staffType, colour); + if (!ok) { + *ebx = MONEY32_UNDEFINED; + return; + } FOR_ALL_PEEPS(spriteIndex, peep) { if (peep->type == PEEP_TYPE_STAFF && peep->staff_type == staffType) { @@ -384,6 +389,11 @@ void game_command_set_staff_order(int *eax, int *ebx, int *ecx, int *edx, int *e if(order_id & 0x80){ // change costume uint8 sprite_type = order_id & ~0x80; sprite_type += 4; + if (sprite_type > countof(peep_slow_walking_types)) { + log_error("Invalid change costume order for sprite_type %u", sprite_type); + *ebx = MONEY32_UNDEFINED; + return; + } peep->sprite_type = sprite_type; peep->peep_flags &= ~PEEP_FLAGS_SLOW_WALK; if(peep_slow_walking_types[sprite_type]){ @@ -1380,7 +1390,7 @@ colour_t staff_get_colour(uint8 staffType) } } -void staff_set_colour(uint8 staffType, colour_t value) +bool staff_set_colour(uint8 staffType, colour_t value) { switch (staffType) { case STAFF_TYPE_HANDYMAN: @@ -1393,7 +1403,7 @@ void staff_set_colour(uint8 staffType, colour_t value) gStaffSecurityColour = value; break; default: - assert(false); - break; + return false; } + return true; } diff --git a/src/peep/staff.h b/src/peep/staff.h index acf16bd10e..70182108af 100644 --- a/src/peep/staff.h +++ b/src/peep/staff.h @@ -87,6 +87,6 @@ bool staff_is_patrol_area_set(int staffIndex, int x, int y); void staff_set_patrol_area(int staffIndex, int x, int y, bool value); void staff_toggle_patrol_area(int staffIndex, int x, int y); colour_t staff_get_colour(uint8 staffType); -void staff_set_colour(uint8 staffType, colour_t value); +bool staff_set_colour(uint8 staffType, colour_t value); #endif diff --git a/src/world/balloon.c b/src/world/balloon.c index 0cff7ed7cb..e8ac4d92f3 100644 --- a/src/world/balloon.c +++ b/src/world/balloon.c @@ -102,12 +102,17 @@ static void balloon_press(rct_balloon *balloon) void game_command_balloon_press(int* eax, int* ebx, int* ecx, int* edx, int* esi, int* edi, int* ebp) { - int balloon_num = *eax; + unsigned int balloon_num = *eax; int flags = *ebx; *ebx = 0; if (!(flags & GAME_COMMAND_FLAG_APPLY)) { return; } + if (balloon_num >= MAX_SPRITES) { + log_error("Tried getting invalid sprite for balloon: %u", balloon_num); + *ebx = MONEY32_UNDEFINED; + return; + } rct_sprite* sprite = get_sprite(balloon_num); if (!sprite) { return;