Phoenix-Protocol-Group / phoenix-contracts

Source code of the smart contracts of the Phoenix DeFi hub DEX protocol
GNU General Public License v3.0
10 stars 6 forks source link

PHOAM-024: Unsafe integer casting could allow adversaries to drain rewards #357

Closed gangov closed 1 month ago

gangov commented 2 months ago

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.