code-423n4 / 2023-03-neotokyo-findings

4 stars 0 forks source link

# configureLP function should check LP stakers present before changing LP address. #373

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1708

Vulnerability details

configureLP function should check LP stakers existence before changing LP address.

Permitted users are allowed to change LP address when lpLocked is false. So this does not follow the comments above.

Proof of concept

1701    This function allows a permitted user to configure the LP token contract 
1702    address. Extreme care must be taken to avoid doing this if there are any LP 
1703    stakers, lest staker funds be lost. It is recommended that `lockLP` be 
1704    invoked.

1708    function configureLP (
1709            address _lp
1710        ) external hasValidPermit(UNIVERSAL, CONFIGURE_LP) { //@@ address 0 check
1711            if (lpLocked) {
1712                revert LockedConfigurationOfLP();
1713            }
1714            LP = _lp;
1715        }

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1702

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1708

It's better to check whether there are LP stakers or not before changing its address.

Tools Used

Vs code

Recommended Mitigation Steps

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

    revert LPstakerExist(); 
    }

Check the LP tokens totalPoints and if it is not 0 then revert the configureLP function. Consider adding this if check to configureLP function.

hansfriese commented 1 year ago

Seems to be out of scope. Will decide after the sponsor's review.

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as primary issue

TimTinkers commented 1 year ago

Out of scope, we assume competent admins.

c4-sponsor commented 1 year ago

TimTinkers marked the issue as sponsor disputed

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Out of scope