Neo Tokyo contest - cccz's results

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1155-L1156

Vulnerability details

Impact

NeoTokyoStaker contracts have three calculations where the division is before the multiplication, which can lead to a slight loss of precision.

1. The * _PRECISION here is to prevent precision loss and is ok.

				uint256 share = points * _PRECISION / pool.totalPoints * totalReward;
				uint256 daoShare = share * pool.daoTax / (100 * _DIVISOR);
				share /= _PRECISION;
				daoShare /= _PRECISION;

2. The loss of precision here will result in a loss of points of up to 0.01 * 1e18 LP tokens when the user stake LP tokens.

			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;

3. The loss of precision here will result in points of up to 0.01 * 1e18 LP tokens not being lost when the user withdraws the LP tokens

			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;

Proof of Concept

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1388-L1391 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1155-L1156 https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1623-L1624

Tools Used

None

Recommended Mitigation Steps

Place multiplication before division to reduce loss of precision

-			uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;
+			uint256 points = amount * 100 * timelockMultiplier / 1e18  / _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;
...
-			uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;
+			uint256 points = amount * 100 * lpPosition.multiplier / 1e18 / _DIVISOR;

			// Update the caller's LP token stake.
			lpPosition.amount -= amount;
			lpPosition.points -= points;

			// Update the pool point weights for rewards.
			pool.totalPoints -= points;