search

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

4 stars 0 forks source link

TIMELOCK OPTION'S DURATION AND MULTIPLIER INACCURATELY DECODED #452

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#L962-L963 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1016-L1017 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1140-L1141

Vulnerability details

Impact

The implementation of bitwise operations, i.e. >> and & in decode the timelock option's duration and multiplier does not seem to return results as expected. This could affect all other variables dependent on them.

Proof of Concept

These affect the function logics of _stakeS1Citizen(), _stakeS2Citizen(), and _stakeLP() in similar ways.

For instance, in the code lines below associated with _stakeLP(), it is evident that _timelock will need to be greater than type(uint128).max while smaller than or equal to type(uint256).max in order to have >> accurately operate on _timelock / (2 ** 128). If not, any other number is going to return zero.

File: NeoTokyoStaker.sol#L1140-L1141

        uint256 timelockDuration = _timelock >> 128;
        uint256 timelockMultiplier = _timelock & type(uint128).max;

Assuming that _timelock falls within the acceptable range of numbers aforesaid, _timelock & type(uint128).max is going to break since _timelock would need to be less than or equal to type(uint128).max in order for & to accurately operate. If not, any other number is going to return zero.

Additionally, this is going to have points == 0 and subsequently, stakerLPPosition[msg.sender].points added 0 too. The staker would end up transfering an LP amount to the contract for zero point added.

NeoTokyoStaker.sol#L1155-L1164

Recommended Mitigation Steps

While I do not have any suggested fix here, the developer team will certainly need to look into refactoring the affected code lines to have all encodings implemented as intended.

hansfriese commented 1 year ago

Check #256 for details.

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Invalid