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

4 stars 0 forks source link

User staking an amount of `BYTES` too low for bonus essentially freezes their `BYTES` for the timelock period #425

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

A user can stake BYTES 2.0 tokens into their Citizens to boost the points weight of those Citizens. The boost (bonusPoints) computed in _stakeBytes():

File: contracts/staking/NeoTokyoStaker.sol
1076:           unchecked {
1077:               uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);
1078:               citizenStatus.stakedBytes += amount;
1079:               citizenStatus.points += bonusPoints;
1080:               pool.totalPoints += bonusPoints;
1081:           }

The rate is 200 BYTES per point. The issue is that if a user calls stake(), transferring an amount lower than 2 BYTES, bonusPoints will be rounded to 0. The variables are updated, but the user does not get any bonus, and has essentially locked their BYTES into the contract for nothing.

Impact

As the only way for a user to retrieve their staked BYTES is to withdraw() their staked Citizen, this means the result is the user's BYTES are frozen until the end of the timelock.

Tools Used

Manual Analysis

Mitigation

In the case of staking BYTES, the call should revert if there is no bonusPoints, to prevent users from freezing temporarily their BYTES without getting any bonus.

File: contracts/staking/NeoTokyoStaker.sol
1076:           unchecked {
1077:               uint256 bonusPoints = (amount * 100 / _BYTES_PER_POINT);
+               if (bonusPoint == 0) revert NoBonus();
1078:               citizenStatus.stakedBytes += amount;
1079:               citizenStatus.points += bonusPoints;
1080:               pool.totalPoints += bonusPoints;
1081:           }
c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #304

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #261

c4-judge commented 1 year ago

hansfriese marked the issue as not a duplicate

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #304

c4-judge commented 1 year ago

hansfriese changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

hansfriese marked the issue as grade-b