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

Lack of address(0) in the KatanaGovernance::_setFactory function #107

Closed
c4-bot-10 opened this issue Oct 24, 2024 · 1 comment
Closed

Lack of address(0) in the KatanaGovernance::_setFactory function #107

c4-bot-10 opened this issue Oct 24, 2024 · 1 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 duplicate-25 🤖_10_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-10
Copy link
Contributor

Lines of code

https://github.com/ronin-chain/katana-operation-contracts/blob/27f9d28e00958bf3494fa405a8a5acdcd5ecdc5d/src/governance/KatanaGovernance.sol#L340

Vulnerability details

Proof of Concept

The KatanaGovernance::_setFactory function is used to change the value of _v2Factory but depending on the current implementation, this variable can take on an address(0) which is an invalid address.
If _v2Factory is address(0) or is not defined correctly, calling the KatanaGovernance::createPair function will always fail and we will get other unexpected behaviour.

    function _setFactory(address factory) private {
        // @audit lack of address(0) check
        _v2Factory = IKatanaV2Factory(factory);

        emit FactoryUpdated(_msgSender(), factory);
    }

Impact

DOS and unattended behaviour, as some functions will always fail due to an invalid address.

Recommended Mitigation Steps

    function _setFactory(address factory) private {
+       require(factory != address(0), "Invalid address!");
        _v2Factory = IKatanaV2Factory(factory);

        emit FactoryUpdated(_msgSender(), factory);
    }

Assessed type

Invalid Validation

@c4-bot-10 c4-bot-10 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 24, 2024
c4-bot-4 added a commit that referenced this issue Oct 24, 2024
@c4-bot-12 c4-bot-12 added the 🤖_10_group AI based duplicate group recommendation label Oct 30, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@howlbot-integration howlbot-integration bot added sufficient quality report This report is of sufficient quality duplicate-25 labels Nov 4, 2024
@nevillehuang
Copy link

nevillehuang commented Nov 4, 2024

Note, this are not present in the automated finding report for known issues. Grouping all address(0) input validation checks together. They should all likely be at most QA and argubly invalid based on the following READ.ME information:

Centralization risk. Sky Mavis is responsible for maintaining the Katana V3 contracts and will able to upgrade the contract if necessary, as well as specify additional fee tiers.

#101 would require a user error.

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 duplicate-25 🤖_10_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants