Skip to content

Commit

Permalink
Prevent undefined behaviour modifying bionic power
Browse files Browse the repository at this point in the history
As it turns out, the check to prevent integer overflow when modifying
bionic power is undefined behaviour. Whoops! Let's avoid that.
  • Loading branch information
anothersimulacrum committed Jan 4, 2020
1 parent ed9e547 commit c30844c
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
10 changes: 10 additions & 0 deletions data/json/mutations/mutations.json
Original file line number Diff line number Diff line change
Expand Up @@ -6437,6 +6437,16 @@
"debug": true,
"active": true
},
{
"type": "mutation",
"id": "DEBUG_BIONIC_POWERGEN",
"name": "Debug Bionic Powergen",
"points": 99,
"valid": false,
"description": "Activate to increase power by an amount you specify (can be repeated).",
"debug": true,
"active": true
},
{
"type": "mutation",
"id": "SQUEAMISH",
Expand Down
16 changes: 10 additions & 6 deletions src/character.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1389,13 +1389,17 @@ void Character::set_max_power_level( units::energy npower_max )

void Character::mod_power_level( units::energy npower )
{
int new_power = units::to_millijoule( power_level ) + units::to_millijoule( npower );
// An integer overflow has occurred - the sum of two positive things (power_level is always positive)
// cannot be negative, so if it is, we know integer overflow has occured
if( npower >= 0_mJ && new_power < 0 ) {
new_power = units::to_millijoule( max_power_level );
// units::energy is an int, so avoid overflow by converting it to a int64_t, then adding them
// If the result is greater than the max power level, set power to max
int64_t power = static_cast<int64_t>( units::to_millijoule( power_level ) ) +
static_cast<int64_t>( units::to_millijoule( npower ) );
units::energy new_power;
if( power > units::to_millijoule( max_power_level ) ) {
new_power = max_power_level;
} else {
new_power = power_level + npower;
}
power_level = clamp( units::from_millijoule( new_power ), 0_kJ, max_power_level );
power_level = clamp( new_power, 0_kJ, max_power_level );
}

void Character::mod_max_power_level( units::energy npower_max )
Expand Down
10 changes: 10 additions & 0 deletions src/mutation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "mapdata.h"
#include "memorial_logger.h"
#include "monster.h"
#include "output.h"
#include "overmapbuffer.h"
#include "player.h"
#include "translations.h"
Expand Down Expand Up @@ -53,6 +54,7 @@ static const trait_id trait_TREE_COMMUNION( "TREE_COMMUNION" );
static const trait_id trait_ROOTS2( "ROOTS2" );
static const trait_id trait_ROOTS3( "ROOTS3" );
static const trait_id trait_DEBUG_BIONIC_POWER( "DEBUG_BIONIC_POWER" );
static const trait_id trait_DEBUG_BIONIC_POWERGEN( "DEBUG_BIONIC_POWERGEN" );

namespace io
{
Expand Down Expand Up @@ -552,6 +554,14 @@ void Character::activate_mutation( const trait_id &mut )
add_msg_if_player( m_good, _( "Bionic power storage increased by 100." ) );
tdata.powered = false;
return;
} else if( mut == trait_DEBUG_BIONIC_POWERGEN ) {
int npower;
if( query_int( npower, "Modify bionic power by how much? (Values are in millijoules)" ) ) {
mod_power_level( units::from_millijoule( npower ) );
add_msg_if_player( m_good, "Bionic power increased by %dmJ.", npower );
tdata.powered = false;
}
return;
} else if( !mdata.spawn_item.empty() ) {
item tmpitem( mdata.spawn_item );
i_add_or_drop( tmpitem );
Expand Down

0 comments on commit c30844c

Please sign in to comment.