Lines of code
<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1155>
<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1623>
Precision issue leading to zero truncation due to numerator smaller than denominator in a ratio or a division happens readily in Solidity if extra cares have not been given to it. Arithmetic operations running into this incident are typically associated with division before multiplication and/or inadequate scaling of the numerator.
There are a total of 3 instances found in NeoTokyoStaker.sol.
uint256 constant private _BYTES_PER_POINT = 200 * 1e18;
NeoTokyoStaker.sol#L1077-L1080
uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);
citizenStatus.stakedBytes += amount;
citizenStatus.points += bonusPoints;
pool.totalPoints += bonusPoints;
Note: The decimal number of BYTES, as inherited from Openzeppelin ERC20.sol, is 18.
If amount were inputted as 2e18 - 1,
bonusPoints = (2e18 - 1) * 100 / (200 * 1e18) = (200e18 - 100) / 200e18 = 0
As a result,
citizenStatus.stakedBytes = citizenStatus.stakedBytes + (2e18 - 1)
citizenStatus.points = citizenStatus.points + 0
pool.totalPoints = pool.totalPoints + 0
The staker ended up transferring (2e18 - 1) BYTES to the contract with zero point added to boost his/her reward emission in the S1 or S2 Citizen Pool.
NeoTokyoStaker.sol#L1155-L1164
uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;
// Update the caller's LP token stake.
stakerLPPosition[msg.sender].timelockEndTime =
block.timestamp + timelockDuration;
stakerLPPosition[msg.sender].amount += amount;
stakerLPPosition[msg.sender].points += points;
// Update the pool point weights for rewards.
pool.totalPoints += points;
Assumption: The decimal number of LP is also 18, as inherited from Openzeppelin ERC20.sol.
If amount were inputted as 1e16 - 1,
points = (1e16 - 1) * 100 / 1e18 * timelockMultiplier / _DIVISOR
= (1e18 - 100) / 1e18 * timelockMultiplier / _DIVISOR
= 0 * * timelockMultiplier / _DIVISOR = 0
As a result,
stakerLPPosition[msg.sender].amount = stakerLPPosition[msg.sender].amount + (1e16 - 1)
stakerLPPosition[msg.sender].points = stakerLPPosition[msg.sender].points + 0
The staker ended up transferring (1e16 - 1) LP to the contract with zero point added to boost his/her reward emission in the LP Pool.
NeoTokyoStaker.sol#L1623-L1630
uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;
// Update the caller's LP token stake.
lpPosition.amount -= amount;
lpPosition.points -= points;
// Update the pool point weights for rewards.
pool.totalPoints -= points;
Again, if amount were inputted as 1e16 - 1,
points = (1e16 - 1) * 100 / 1e18 * lpPosition.multiplier / _DIVISOR
= (1e18 - 100) / 1e18 * lpPosition.multiplier / _DIVISOR
= 0 * * lpPosition.multiplier / _DIVISOR = 0
As a result,
lpPosition.amount = lpPosition.amount - (1e16 - 1)
lpPosition.points = lpPosition.points - 0
This vulnerability actually works in great favor to the staker. Specifically, the staker could repeatedly perform the exploit to cumulatively withdraw his/her LP tokens without deducting any points from his/her LP position.
Since stakerLPPosition never gets reset, it does not matter whether or not the stakerβs lpPosition.amount remains non-zero or equals zero. Apparently, the staker could restake the withdrawn LPs (of course with an amount greater than 0.01 LP tokens) to keep boosting the rewards emission in the LP Pool.
It is recommended making the first multiplicand, i.e. amount, scaled to a higher order. In the case of the LP pool, the arithmetic operations could also benefit from simplifying two divisions to one division.
The text was updated successfully, but these errors were encountered:
All reactions