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 #14

Closed
howlbot-integration bot opened this issue Nov 4, 2024 · 3 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 issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

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

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_primary AI based primary recommendation bug Something isn't working edited-by-warden primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Nov 4, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@khangvv
Copy link

khangvv commented Nov 4, 2024

The NonfungiblePositionManager contract functions as a transparent proxy, leaving its private state variables uninitialized (set to 0). Therefore, the initialize function is necessary and will succeed on its first call.

@khangvv khangvv added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 4, 2024
@alex-ppg
Copy link

This submission is similar to the #26 issue class and the same rationale applies here.

@c4-judge
Copy link

alex-ppg marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 11, 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 issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants