hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

Smart contracts for the Metrom project.
GNU General Public License v3.0
0 stars 0 forks source link

Irreversible Contract Ossification #62

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7d9a15d6f211ead3f63e866a3efe8ee292765d7b8222d666872a80d9db660181 Severity: medium

Description: Description\ The ossify function in the Metrom contract allows the owner to disable the ability to upgrade the contract. Once this function is called, the ossified state is set to true, and the _authorizeUpgrade function will always revert if an upgrade is attempted. This can be problematic if the function is called mistakenly or if future upgrades are necessary to fix bugs or add features. A more flexible approach would be to allow the owner to pass a parameter to control the ossified state, enabling the possibility to revert the ossification if needed.

Attack Scenario\ permanently preventing any future upgrades to the contract. This could hinder the ability to patch vulnerabilities or add new features, potentially leading to significant financial and operational risks.

Attachments

  1. Proof of Concept (PoC) File

    https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L86C5-L90C6

https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L92C5-L95C6

  1. Revised Code File (Optional)

    function ossify(bool _ossified) external {
        if (msg.sender != owner) revert Forbidden();
        ossified = _ossified;
        emit Ossify(_ossified);
    }

    The revised ossify function now accepts a boolean parameter _ossified that allows the owner to set the ossified state to either true or false. This change provides flexibility to revert the ossification if needed, thus allowing future upgrades to the contract. The emit Ossify(_ossified); line ensures that the event logs the new state of the ossified variable.

Files:

luzzif commented 5 months ago

This is actually intended behavior.