PHOAM-024: Unsafe integer casting could allow adversaries to drain rewards
Closed this issue · 0 comments
Location
./contracts/stake/src/distribution.rs
Description
An unsafe cast of an i128
number to u128
could cause the withdrawable_rewards
function to return an excessively large number, potentially allowing an adversary to drain the rewards balance.
In the code below, the amount as u128
snippet casts an i128
number into a u128
. While casting a positive i128
to u128
has no side effect, casting a negative i128
number, N
, to u128
results in the value u128::MAX - N
. This unintended behavior is not meant to occur in the withdrawable_rewards function.
let ppw = distribution.shares_per_point;
let stakes: i128 = get_stakes(env, owner).total_stake;
// Decimal::one() represents the standart multiplier per token
// 1_000 represents the contsant token per power. TODO: make it
configurable
let points = calc_power(config, stakes as i128, Decimal::one(),
TOKEN_PER_POWER);
let points = (ppw * points as u128) as i128;
let correction = adjustment.shares_correction;
let points = points + correction;
let amount = points >> SHARES_SHIFT;
amount as u128 - adjustment.withdrawn_rewards
Coinspect consultants analyzed various scenarios to exploit this issue by having a negative correction absolute value greater than points, but found no successful outcomes. As a result, this issue is assessed as informational. Due to the time constraints of this engagement, it is left to the Phoenix team to further analyze the feasibility of this scenario.
Recommendation
Verify that the amount variable cannot hold a negative number and add a failsafe mechanism to make withdrawable_rewards
throw an error in such case.
Additionally, review the rest of the code for any potentially dangerous integer casts.