_withdrawLP is not re-setting the lpPosition.points when lpPosition.amount

[code423n4]

Lines of code

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

Vulnerability details

Impact

User can withdraw their LP tokens without affecting their lpPosition.points. Since the lpPosition.points could not deducted then and there whenever the LP token is drawn out, user can use the old lpPosition.points and new lpPosition.points value to calculate the share vale when staking the LP tokens. This would result unfair amount of share even if they are not entitled for it. Loss of funds.

Proof of Concept

User can stake the LP tokens using the function _stakeLP()

    PoolData storage pool = _pools[AssetType.LP];
    unchecked {
        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;
    }

stakerLPPosition[msg.sender].points value is updated based on the points value calculated from uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;

when claiming the reward using the claimReward(), reward amount will be calculated using getPoolReward() When we look at the LP token code snip,

        } else if (_assetType == AssetType.LP) {
            unchecked {
                points += stakerLPPosition[_recipient].points;
            }
        } else {

and this points values are used to calculated the share value.

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

When we look at the _withdrawLP () function, for the given amount, the points value is calculated using, uint256 points = amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR;, It is possible for a user to call this function with input amount value which would give zero points from above calculation, malicious user can call this function multiple times without affecting their point values. After withdrawing entire staked amount, when they come back for stake, old points value could be used to calculated the share value.

Tools Used

Manual review.

Recommended Mitigation Steps

Following suggestion is proposed.

For the given amount, make sure that the valid amount of points are deducted.

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #348

[c4-judge]

hansfriese marked the issue as duplicate of #261