Lines of code
<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1264-L1396&gt;

Vulnerability details

Impact

When withdrawing the staked LP tokens, the staker can divide the total staked token amount into smaller amounts and call the NeoTokyoStaker.withdraw function, which further calls the following NeoTokyoStaker._withdrawLP function, to withdraw each of such smaller token amounts. When such token amount is small enough, executing uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR in the NeoTokyoStaker._withdrawLP function can cause points to equal 0, which does not decrease the staker’s lpPosition.points at all. In this situation, given some time between two NeoTokyoStaker.withdraw function calls, the staker can benefit from the unchanged lpPosition.points in each subsequent NeoTokyoStaker.withdraw function call, which eventually calls the lpPosition.getPoolReward function below. When executing uint256 share = points * _PRECISION / pool.totalPoints * totalReward in each lpPosition.getPoolReward function call, the unchanged lpPosition.points would cause a positive share to be minted to the staker while the staker should deserve 0 extra reward shares if it has withdrawn the staked LP tokens all at once and reduced its lpPosition.points to 0. As a result, the total number of reward shares minted to the staker becomes more than the staker is entitled to.

<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1597-L1644&gt;

	function _withdrawLP () private {
		uint256 amount;
		assembly{
			amount := calldataload(0x24)
		}

		...
		_assetTransfer(LP, msg.sender, amount);

		// Update caller staking information and asset data.
		PoolData storage pool = _pools[AssetType.LP];
		unchecked {
			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;
		}

		...
	}

<https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1264-L1396&gt;

	function getPoolReward (
		AssetType _assetType,
		address _recipient
	) public view returns (uint256, uint256) {

		...
		PoolData storage pool = _pools[_assetType];
		if (pool.totalPoints != 0) {

			// Calculate the total number of points accrued to the _recipient.
			uint256 points;
			if (_assetType == AssetType.S1_CITIZEN) {
				...
			} else if (_assetType == AssetType.S2_CITIZEN) {
				...
			} else if (_assetType == AssetType.LP) {
				unchecked {
					points += stakerLPPosition[_recipient].points;
				}
			} else {
				revert InvalidAssetType(uint256(_assetType));
			}

			...

			// Return final shares.
			unchecked {
				uint256 share = points * _PRECISION / pool.totalPoints * totalReward;
				uint256 daoShare = share * pool.daoTax / (100 * _DIVISOR);
				share /= _PRECISION;
				daoShare /= _PRECISION;
				return ((share - daoShare), daoShare);
			}
		}
		return (0, 0);
	}

Proof of Concept

The following steps can occur for the described scenario.

1. The staker has staked 0.9e17 LP tokens. Since these staked tokens’ end time is reached, the staker wants to withdraw them. Instead of withdrawing the 0.9e17 LP tokens all at once, it decides to withdraw 0.9e16 LP tokens at a time for ten times.
2. When the NeoTokyoStaker.withdraw function is called to withdraw 0.9e16 LP tokens, the lpPosition.getPoolReward function is eventually called. share that is (points * _PRECISION / pool.totalPoints * totalReward) / _PRECISION is minted to the staker.
3. Then, when the NeoTokyoStaker.withdraw function calls the NeoTokyoStaker._withdrawLP function, lpPosition.multiplier can be 100, and uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR is evaluated to points = 0.9e16 * 100 / 1e18 * 100 / 100 = 0. Therefore, the staker’s lpPosition.points is not decreased at all.
4. After some time, when the NeoTokyoStaker.withdraw function is called again to withdraw another 0.9e16 LP tokens, the lpPosition.getPoolReward function is called again, and a positive share, which is (points * _PRECISION / pool.totalPoints * totalReward) / _PRECISION, is minted to the staker because the staker’s lpPosition.points did not change and was not reduced to 0.
5. Steps 3 and 4 can be repeated until the total 0.9e17 staked LP tokens are all withdrawn. In this way, the extra reward shares, which the staker is not entitled to, are minted to the staker.
6. If the staker withdraws the 0.9e17 staked LP tokens all at once, its lpPosition.points would be reduced to 0 when the NeoTokyoStaker._withdrawLP function is called, and any subsequent lpPosition.getPoolReward function call would calculate the share to be 0 so no extra reward shares would be minted to the staker. However, such extra reward shares that the staker should not deserve are minted to the staker for the described scenario.

Tools Used

VSCode

Recommended Mitigation Steps

The NeoTokyoStaker._withdrawLP function can be updated to revert when the calculated points is 0 to ensure that the staker must withdraw enough staked LP tokens to reduce its lpPosition.points properly.

The text was updated successfully, but these errors were encountered:

All reactions