hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

`Ownable2Step` should be used instead of `Ownable` in `packages/contracts/contracts/illuminex/xengine/chains/btc/AllowedRelayers.sol`. #2

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

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

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

Description: Description

The Ownable contract can be upgraded to Open Zeppelin's Ownable2Step: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol.

Ownable2Step provides added safety due to its securely designed two-step process.

Attack Scenario

If wrong address is set, owner cannot be recovered.

Attachments

NA

  1. Proof of Concept (PoC) File

In packages/contracts/contracts/illuminex/xengine/chains/btc/AllowedRelayers.sol:

abstract contract AllowedRelayers is Ownable {

Ownable is used instead of Ownable2Step.

  1. Revised Code File (Optional)

It is a best practice to use two-step ownership transfer pattern, meaning ownership transfer gets to a "pending" state and the new owner should claim his new rights, otherwise the old owner still has control of the contract. Consider using OpenZeppelin's Ownable2Step contract.

rotcivegaf commented 4 months ago

Non issue