Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require nextId to be zero will always revert due to nextId already initialized to 1 #143

Open
c4-bot-3 opened this issue Oct 28, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

c4-bot-3 commented Oct 28, 2024

Lines of code

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/periphery/NonfungiblePositionManager.sol#L104

Vulnerability details

In NonfungiblePositionManager.sol there will be a problem initializing.

  /// @dev The ID of the next token that will be minted. Skips 0
  uint176 private _nextId = 1;
  /// @dev The ID of the next pool that is used for the first time. Skips 0
  uint80 private _nextPoolId = 1;

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/periphery/NonfungiblePositionManager.sol#L83

_nextId is initialized to 1 in state variable declaration

  function initialize() external {
    require(_nextId == 0, "Already initialized");

    _nextId = 1;
    _nextPoolId = 1;
  }

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/periphery/NonfungiblePositionManager.sol#L104

Initialize function checks if it's 0

This means that the check will always fail to initialize

Recommended Mitigation Steps

  function initialize() external {
    require(_nextId == 1, "Already initialized");

    _nextId = 1;
    _nextPoolId = 1;
  }

Chnage the require statement to check for _nextId == 1

Assessed type

Error

@c4-bot-3 c4-bot-3 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 28, 2024
c4-bot-4 added a commit that referenced this issue Oct 28, 2024
@c4-bot-12 c4-bot-12 added the 🤖_primary AI based primary recommendation label Oct 30, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants