hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

usage of single step ownership transfer instead of 2 step #33

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xef009bd8fdcf6d6be29e78d75ba6432831992897b61af724fda562f4f3d66527 Severity: low

Description: Description\ Single-Step ownership transfers are dangerous as if the transfer is made to an incorrect address, the contract will be with no owner, and the role will be lost forever.
That's why 2 step ownership transfers are preferred and recommended as best practice. They can prevent such scenarios from happening. 2-step 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.

For Thorn, all those contracts use single step ownership transfer, and the potential issue is present. They inherit Ownable

StableSwapFactory, StableSwapRouter, StableSwapLPFactory, StableSwapThreePoolDeployer, StableSwapTwoPoolDeployer, StableSwapThreePool, StableSwapTwoPool

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

Use OpenZeppelin's Ownable2Step instead of Ownable https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol

omega-audits commented 2 months ago

This issue describes a best practice, but not an exploitable vulnerability.