During ownership transfer across protocol contracts, the previous owner isn't cleared from the `isAdmin` mapping

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/Neuron.sol#L1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/RankedBattle.sol#L1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/VoltageManager.sol#L1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/MergingPool.sol#L1 https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L1

Vulnerability details

Impact

The issue of not clearing the previous owner from the isAdmin mapping during ownership transfers presents a significant security risk across multiple contracts in the AI Arena protocol. This oversight allows former owners to retain administrative privileges, potentially enabling unauthorized access and manipulation of contract functionalities. Such vulnerabilities compromise the security, integrity, and trustworthiness of the protocol, posing risks to both the platform's operations and its users' interests.

Proof of Concept

• Neuron.sol
• RankedBattle.sol
• GameItems.sol
• VoltageManager.sol
• MergingPool.sol
• FighterFarm.sol

The contracts listed above are integral to the AI Arena protocol, each containing functions for ownership transfer and administrative privilege management. The absence of code to explicitly remove the previous owner's admin status in these contracts can lead to scenarios where former owners can still execute actions meant solely for the current owner or administrators.

For example, a typical ownership transfer function without properly updating the isAdmin mapping in AI Aren contracts look like:

    function transferOwnership(address newOwnerAddress) external {
        require(msg.sender == _ownerAddress);
        _ownerAddress = newOwnerAddress;
    }

This pattern creates a uniform vulnerability.

Tools Used

• Manual Review

Recommended Mitigation Steps

ensure that any function that transf ownership includes steps to revoke the admin privileges of previous owner.

    function transferOwnership(address newOwnerAddress) external {
        require(msg.sender == _ownerAddress);
        isAdmin[_ownerAddress] = false;
        _ownerAddress = newOwnerAddress;
    }

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #2

[c4-judge]

HickupHH3 changed the severity to QA (Quality Assurance)