From fc4fb735d383adafdd697c1d40ff6331416694e0 Mon Sep 17 00:00:00 2001 From: BevapDin Date: Thu, 7 May 2020 18:46:15 +0200 Subject: [PATCH] Fix crash caused by accessing item of empty list. The function `are_requirements_nearby` accesses `player::backlog::front()`, which causes UB when the `backlog` list is empty. This is usually checked by the caller, but there is at least one way to reach this function without that check: ``` #2 0x000000000083dfaa in string_id::operator== (rhs=..., this=0xdf1b460) at src/player_activity.h:93 #3 are_requirements_nearby (loot_spots=std::vector of length 21, capacity 32 = {...}, needed_things=..., p=..., activity_to_restore=..., in_loot_zones=true, src_loc=...) at src/activity_item_handling.cpp:1113 #4 0x0000000000843a46 in generic_multi_activity_check_requirement (p=..., act_id=..., act_info=..., src=..., src_loc=..., src_set=std::unordered_set with 1 element = {...}, check_only=false) at src/activity_item_handling.cpp:2676 #5 0x0000000000852813 in generic_multi_activity_handler (act=..., p=..., check_only=check_only@entry=false) at src/activity_item_handling.cpp:2885 #6 0x0000000000800896 in activity_handlers::multiple_butcher_do_turn (act=, p=) at src/activity_handlers.cpp:3822 #7 0x000000000082b248 in std::_Function_handler::_M_invoke(std::_Any_data const&, player_activity*&&, player*&&) (__functor=..., __args#0=, __args#1=) at /usr/include/c++/8/bits/std_function.h:88 #8 0x00000000008630e3 in std::function::operator()(player_activity*, player*) const (this=, __args#0=, __args#0@entry=0xdf1b330, __args#1=, __args#1@entry=0xdf1ae50) at /usr/include/c++/8/bits/std_function.h:260 #9 0x0000000000860306 in activity_type::call_do_turn (this=0x2c3c930, act=act@entry=0xdf1b330, p=p@entry=0xdf1ae50) at src/activity_type.cpp:118 #10 0x00000000014a968b in player_activity::do_turn (this=this@entry=0xdf1b330, p=...) at src/player_activity.cpp:237 #11 0x00000000013092a4 in npc::do_player_activity (this=this@entry=0xdf1ae50) at src/npcmove.cpp:3299 #12 0x0000000001322a07 in npc::execute_action (this=this@entry=0xdf1ae50, action=, action@entry=npc_player_activity) at src/npcmove.cpp:1237 #13 0x000000000132690a in npc::move (this=this@entry=0xdf1ae50) at src/npcmove.cpp:907 ``` This adds a simple check within the function. --- src/activity_item_handling.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/activity_item_handling.cpp b/src/activity_item_handling.cpp index 071bdfc9d7578..e5f6a1eff9170 100644 --- a/src/activity_item_handling.cpp +++ b/src/activity_item_handling.cpp @@ -984,22 +984,18 @@ static bool are_requirements_nearby( const std::vector &loot_spots, inventory temp_inv; units::volume volume_allowed = p.volume_capacity() - p.volume_carried(); units::mass weight_allowed = p.weight_capacity() - p.weight_carried(); - const bool check_weight = p.backlog.front().id() == ACT_MULTIPLE_FARM || - activity_to_restore == ACT_MULTIPLE_FARM || - p.backlog.front().id() == ACT_MULTIPLE_CHOP_PLANKS || - activity_to_restore == ACT_MULTIPLE_CHOP_PLANKS || - p.backlog.front().id() == ACT_MULTIPLE_BUTCHER || - activity_to_restore == ACT_MULTIPLE_BUTCHER || - p.backlog.front().id() == ACT_VEHICLE_DECONSTRUCTION || - activity_to_restore == ACT_VEHICLE_DECONSTRUCTION || - p.backlog.front().id() == ACT_VEHICLE_REPAIR || - activity_to_restore == ACT_VEHICLE_REPAIR || - p.backlog.front().id() == ACT_MULTIPLE_CHOP_TREES || - activity_to_restore == ACT_MULTIPLE_CHOP_TREES || - p.backlog.front().id() == ACT_MULTIPLE_FISH || - activity_to_restore == ACT_MULTIPLE_FISH || - p.backlog.front().id() == ACT_MULTIPLE_MINE || - activity_to_restore == ACT_MULTIPLE_MINE; + static const auto check_weight_if = []( const activity_id & id ) { + return id == ACT_MULTIPLE_FARM || + id == ACT_MULTIPLE_CHOP_PLANKS || + id == ACT_MULTIPLE_BUTCHER || + id == ACT_VEHICLE_DECONSTRUCTION || + id == ACT_VEHICLE_REPAIR || + id == ACT_MULTIPLE_CHOP_TREES || + id == ACT_MULTIPLE_FISH || + id == ACT_MULTIPLE_MINE; + }; + const bool check_weight = check_weight_if( activity_to_restore ) || ( !p.backlog.empty() && + check_weight_if( p.backlog.front().id() ) ); bool found_welder = false; for( item *elem : p.inv_dump() ) { if( elem->has_quality( qual_WELD ) ) {