Neo Tokyo contest - anodaram's results

Lines of code

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

Vulnerability details

Impact

This example shows us one tricky in math floor function.

1.51 + 1.51 - 1.00 - 1.00 - 1.00 > 0
f(1.51) + f(1.51) - f(1.00) - f(1.00) - f(1.00) < 0 

// f is math floor function

To make it easy to describe, let's say that timelockMultiplier is 100.

points calculation part of stake/withdraw LP are on Line 1155 and Line 1623.

uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;

So, amount = 1.51 * 10**16 => points = 1 amount = 1.00 * 10**16 => points = 1

An example of a malicious user's activity.

After Step 5, the points should be -1. Since the datatype is unsinged integer, it will be INFINITY.

This can cause several critical attacks. In line 1388-1392, which is calculating user share amount in function getPoolReward

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

(We can assume that pool.totalPoints is not negative because we have other users' sharing) In case of points = max(uint256) - 1, share can be unexpected value. The malicious user can getReward huge amount of asset.

Proof of Concept

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

Tools Used

(Nothing, just looking code on VSCode)

Recommended Mitigation Steps

We can recalculate lpPosition.points based on newAmount in _withdrawLP.

Original

	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;

Fixed

	uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;

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

	uint256 newPoints = lpPosition.amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;
	pool.totalPoints -= lpPosition.points - newPoints;
	lpPosition.points = newPoints;

Also, need to fix in the same way in _stakeLP.