search

hats-finance / Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4

Other
1 stars 2 forks source link

Ownable2Step should be used instead of Ownable #8

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

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

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xa49c48bd3ab8fa64f03ec6d56125c5ee14ad5034043da3c1f584ee617382e7cd 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.

For Instance in CatalystFactory.sol:

./CatalystFactory.sol:20:contract CatalystFactory is Ownable, ICatalystV1Factory {

Attack Scenario\

If wrong address is set, owner cannot be recovered.

Attachments

NA

  1. Proof of Concept (PoC) File

NA

  1. Revised Code File (Optional)

    Use Ownable2Step from OpenZeppelin: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol

reednaa commented 6 months ago

Intentional.

0xEricTee commented 6 months ago

@reednaa any reasons behind?

reednaa commented 6 months ago

It contains more code and the contracts are on the border of what can fit: forge compile --sizes