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

Consider implementing two-step procedure for updating owner address in Minter.sol. #5

Open
hats-bug-reporter bot opened this issue Sep 2, 2024 · 1 comment

Comments

@hats-bug-reporter
Copy link

Github username: @erictee2802
Twitter username: 0xEricTee
Submission hash (on-chain): 0xa22a8ba16c60fed9aaedd715ba246648060b3a606983c58acf8b662fe2a800f1
Severity: low

Description:
Description

Implementing a two-step procedure for updating protocol addresses adds an extra layer of security. In such a system, the first step initiates the change, and the second step, after a predefined delay, confirms and finalizes it. This delay allows stakeholders or monitoring tools to observe and react to unintended or malicious changes. If an unauthorized change is detected, corrective actions can be taken before the change is finalized. To achieve this, introduce a "proposed address" state variable and a "delay period". Upon an update request, set the "proposed address". After the delay, if not contested, the main protocol address can be updated.

Attack Scenario

If wrong address is set, owner cannot be recovered and functions with onlyOwner modifier is no longer callable.

Attachments

NA

  1. Proof of Concept (PoC) File

Minter.sol inherits BaseMinter which inherits Ownable contract:

abstract contract BaseMinter is Ownable, ReentrancyGuard {
  1. Revised Code File (Optional)

Implement two-step procedure while changing owner address or use OpenZeppelin's Ownable2Step.sol.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 2, 2024
@0xRizwan
Copy link

0xRizwan commented Sep 2, 2024

This issue is a suggestion/design feature and not a vulnerability. Further, under following contest rule, its OOS.

Issues about the ability for multi-sig owner to set parameters in a way breaking the contract (they are trusted to be both non-malicious and non-stupid)

@0xRizwan 0xRizwan added Invalid - Lead auditor and removed bug Something isn't working labels Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant