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

Irreversible Contract Ossification #62

Open
hats-bug-reporter bot opened this issue Jun 3, 2024 · 1 comment
Open

Irreversible Contract Ossification #62

hats-bug-reporter bot opened this issue Jun 3, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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:

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 3, 2024
@luzzif luzzif added the invalid This doesn't seem right label Jun 5, 2024
@luzzif
Copy link

luzzif commented Jun 5, 2024

This is actually intended behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant