sherlock-audit / 2024-02-perennial-v2-3-judging

6 stars 5 forks source link

bigbick123456789000 - Exploitable Flaw in `transferOwnership` Function #4

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

bigbick123456789000

medium

Exploitable Flaw in transferOwnership Function

Summary

The transferOwnership function inherited from the Ownable contract within the ProxyOwner contract lacks proper access control, allowing any account to call it and transfer ownership without adhering to the intended two-step ownership transfer mechanism.

Vulnerability Detail

The transferOwnership function overrides the same function from both the Ownable and Ownable2Step contracts without implementing the necessary access control checks to enforce the two-step ownership transfer process. This function is meant to transfer ownership of the contract, but it does so without requiring confirmation from both the current owner and the new owner, as per the two-step mechanism implemented elsewhere in the contract. As a result, any account can call this function and become the new owner without following the intended security protocol.

function transferOwnership(address newOwner) public override(Ownable, Ownable2Step) {
    super.transferOwnership(newOwner);
}

Impact

The lack of proper access control in the transferOwnership function undermines the security measures intended to protect ownership transfer within the contract. This vulnerability allows unauthorized parties to take over ownership of the contract, potentially leading to malicious actions or unauthorized modifications to critical contract functionality.

Code Snippet

#L47-L49

Tool used

Manual Review

Recommendation

It's essential to override the transferOwnership function in the ProxyOwner contract and enforce the two-step ownership transfer mechanism.

sherlock-admin4 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, the functon simply calls itself in superclass, where there is onlyOwner modifier

nevillehuang commented 4 months ago

Invalid, checked here by the onlyOwner modifier within OZ's Ownable2Step.sol