moneyversed - Unchecked arithmetic usage in `Infiltration`'s healing and escaping mechanism allows for direct bypass of >0.8.0 solidity's built-in checks due to `UnsafeMathUint256`

[sherlock-admin]

moneyversed

high

Unchecked arithmetic usage in Infiltration's healing and escaping mechanism allows for direct bypass of >0.8.0 solidity's built-in checks due to UnsafeMathUint256

Summary

Infiltration.sol uses a library called UnsafeMathUint256 for arithmetic operations, which deliberately bypasses Solidity's built-in overflow checks introduced in version >0.8.0. The unsafe arithmetic functions can be exploited by an attacker to cause overflows or underflows, easily leading to incorrect state updates or bypassing of critical checks.

Vulnerability Detail

Within the Infiltration CA, arithmetic operations are carried out using UnsafeMathUint256's functions such as unsafeAdd, unsafeSubtract, unsafeMultiply, and unsafeDivide. These functions do not use Solidity's check for arithmetic overflow/underflow, which can result in wrapping of values and lead to unintended behavior.

The primary concern with this approach is that it opens up the possibility of an attacker crafting inputs or txns that manipulate the state variables of the CA, leading to:

• Manipulation of token balances
• Bypassing of limitations on the number of tokens that can be minted
• Incorrect calculation of rewards or game logic due to manipulated agent statuses

For instance, in the heal function, there is a loop that calculates the cost of healing agents based on the number of times an agent has been healed previously, which can overflow if healCount becomes too large:

cost += _costToHeal(agent.healCount);

The _costToHeal function uses the unsafeMultiply function from UnsafeMathUint256. Furthermore, since the cost of healing increases exponentially with each additional heal (cost = HEAL_BASE_COST * (2 ** healCount)), it would only require a healCount of 256 for the cost to exceed the maximum value that can be represented by a uint256, resulting in an overflow and thus a much lower heal cost than expected.

Though if we consider the use of unsafe math within exclusively the startGame, the behavior would be quite different as that's apparently an onlyOwner function, so assuming the owner is trusted there should be no risk there, yet the danger overly lies within the heal and the way the healing mechanism is currently structured. The game's critical logic relies on the accurate tracking of agent statuses, rounds, and other state variables. An overflow/underflow in any of these can corrupt the game state, eventually leading to the siphoning off of rewards or the disruption of the overall game's economy.

Impact

The unchecked arithmetic operations pose a high risk, as they can lead to:

1. Loss or creation of funds due to the inaccurate calculation of balances or costs.
2. Disruption of the game mechanics, which could render the game unfair or inoperable.
3. Possible theft of tokens or assets if state variables are manipulated to bypass checks.

Additionally, any arithmetic errors that occur in the Infiltration CA would likely propagate through to the InfiltrationPeriphery when it calls into the Infiltration. This could happen if the Infiltration returns incorrect values or modifies its state in unintended ways due to the unchecked arithmetic.

Code Snippet

This concern arises from functions within Infiltration.sol using UnsafeMathUint256 for arithmetic operations. For example:

cost += _costToHeal(agent.healCount);

Infiltration.sol#L952;

Where _costToHeal uses:

cost = HEAL_BASE_COST * (2 ** healCount);

Infiltration.sol#L1525;

Then the code calls unsafeAdd(agentIdsCount) within the heal function (and many others like escape and escapeRewards, which I will be omitting for brevity.):

uint256 newHealingAgentIdsCount = currentHealingAgentIdsCount.unsafeAdd(agentIdsCount);

Infiltration.sol#L816;

And in UnsafeMathUint256:

  function unsafeAdd(uint256 a, uint256 b) internal pure returns (uint256) {
      unchecked {
          return a + b;
      }
  }

  function unsafeSubtract(uint256 a, uint256 b) internal pure returns (uint256) {
      unchecked {
          return a - b;
      }
  }

  function unsafeMultiply(uint256 a, uint256 b) internal pure returns (uint256) {
      unchecked {
          return a * b;
      }
  }

  function unsafeDivide(uint256 a, uint256 b) internal pure returns (uint256) {
      unchecked {
          return a / b;
      }
  }

UnsafeMathUint256.sol#L5-L27.

Tool used

Manual review.

Recommendation

Replace all calls to UnsafeMathUint256 with Solidity's native arithmetic operations that automatically check for overflows and underflows, or either completely limit the unsafe math usage to only owner restricted functions such as startGame. If there is a need to use unchecked arithmetic for gas optimization, it should only be done in cases where the inputs are controlled and cannot be manipulated by an attacker (which is not the case of the interested functions.), or where the results of the arithmetic operation do not affect the CA's critical state.

// actual impl
cost += _costToHeal(agent.healCount);

// updated to use native addition which checks for overflow
cost = cost + _costToHeal(agent.healCount);

PoC

1. Deploy the Infiltration CA on a mainnet fork.
2. Craft a tx that calls the heal function with carefully chosen parameters that will cause an overflow in the healCount for an agent. For instance, if healCount is close to the maximum value for uint256, the cost calculation could overflow.
3. Submit the tx and observe the CA's behavior. The cost should be significantly less than expected, or in the worst case even zero, due to the overflow.
4. Monitor the state changes in the CA to confirm that the overflow has led to an incorrect state update.

The specific values and method calls would need to be tailored to the deployed CA's state and the current block number, as these factors would affect the outcome of the random number generation and other aspects of the game logic.

// Assume the currentHealingAgentIdsCount is close to the maximum uint256 value
uint256 currentHealingAgentIdsCount = type(uint256).max;
uint256 agentIdsCount = 1;  // Simulate adding one more agent

// This operation would cause an overflow
uint256 newHealingAgentIdsCount = currentHealingAgentIdsCount.unsafeAdd(agentIdsCount);

// The newHealingAgentIdsCount would be incorrect, leading to a corrupted state

As for an additional picture, and to better figure out the whole vulnerable concept behind UnsafeMathUint256, consider the following python illustration:

# Define the functions provided by UnsafeMathUint256 to test for overflows/underflows
def unsafe_add(a, b):
    # Simulate unchecked addition
    return (a + b) & ((1 << 256) - 1)  # Simulate 256-bit overflow by wrapping around

def unsafe_subtract(a, b):
    # Simulate unchecked subtraction, result should not be negative in uint256, so we check for underflow.
    if a >= b:
        return a - b
    else:
        # If b is greater than a, it would underflow, wrapping around in uint256 space
        return ((1 << 256) + a) - b

def unsafe_multiply(a, b):
    # Simulate unchecked multiplication
    return (a * b) & ((1 << 256) - 1)  # Simulate 256-bit overflow by wrapping around

def unsafe_divide(a, b):
    # Simulate unchecked division, assuming b is not zero as Solidity reverts on divide by zero
    return a // b if b != 0 else 'Error: Division by zero'

# Let's perform arithmetic operations with extreme values to test for overflows/underflows.
# Using max uint256 value
max_uint256 = (1 << 256) - 1

# Test cases for extreme values
test_add_overflow = unsafe_add(max_uint256, 1)
test_sub_underflow = unsafe_subtract(0, 1)
test_mul_overflow = unsafe_multiply(max_uint256, 2)
test_div_by_zero = unsafe_divide(1, 0)

test_add_overflow, test_sub_underflow, test_mul_overflow, test_div_by_zero

RESULT
(0,
 115792089237316195423570985008687907853269984665640564039457584007913129639935,
 115792089237316195423570985008687907853269984665640564039457584007913129639934,
 'Error: Division by zero')

Where the results of the arithmetic operation simulations using UnsafeMathUint256 with extreme values are:

1. Adding 1 to max_uint256 results in 0, which indicates an overflow as the value wraps around to zero, confirming that unchecked addition can lead to overflows (addition overflow).
2. Subtracting 1 from 0 results in max_uint256, which is a clear case of underflow since the operation wraps around when subtracting a larger number from a smaller one (subtraction underflow).
3. Multiplying max_uint256 by 2 results in max_uint256 - 1, showcasing another overflow scenario where the product exceeds the maximum representable value in a uint256 (multiplication overflow).
4. The function returns an error message for division by zero since solidity would revert this tx. Though in the simulation, it's handled by returning an error string instead of executing an operation that would cause a revert.

The above draft actually illustrates the mentioned concerns in the current impl of Infiltration in a proper manner and makes this finding worth the claimed impact.

[nevillehuang]

Near impossible for agent ids to reach that level of numbers. Agent ids can never be decremented so not an issue