Skip to content

Commit

Permalink
Fix crash caused by accessing item of empty list.
Browse files Browse the repository at this point in the history
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<activity_type>::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=<optimized out>, p=<optimized out>) at src/activity_handlers.cpp:3822
#7  0x000000000082b248 in std::_Function_handler<void (player_activity*, player*), void (*)(player_activity*, player*)>::_M_invoke(std::_Any_data const&, player_activity*&&, player*&&) (__functor=..., __args#0=<optimized out>, __args#1=<optimized out>) at /usr/include/c++/8/bits/std_function.h:88
#8  0x00000000008630e3 in std::function<void (player_activity*, player*)>::operator()(player_activity*, player*) const (this=<optimized out>, __args#0=<optimized out>,
    __args#0@entry=0xdf1b330, __args#1=<optimized out>, __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
CleverRaven#11 0x00000000013092a4 in npc::do_player_activity (this=this@entry=0xdf1ae50) at src/npcmove.cpp:3299
CleverRaven#12 0x0000000001322a07 in npc::execute_action (this=this@entry=0xdf1ae50, action=<optimized out>, action@entry=npc_player_activity) at src/npcmove.cpp:1237
CleverRaven#13 0x000000000132690a in npc::move (this=this@entry=0xdf1ae50) at src/npcmove.cpp:907
```

This adds a simple check within the function.
  • Loading branch information
BevapDin committed May 7, 2020
1 parent 0466c67 commit fc4fb73
Showing 1 changed file with 12 additions and 16 deletions.
28 changes: 12 additions & 16 deletions src/activity_item_handling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -984,22 +984,18 @@ static bool are_requirements_nearby( const std::vector<tripoint> &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 ) ) {
Expand Down

0 comments on commit fc4fb73

Please sign in to comment.