20,000 USDC
View results
Submission Details
Severity: medium
Valid

Single-step process for critical ownership transfer is risky

Summary

Beedle.sol allows owner to mint and Lender.sol allows owner to setLenderFee,setBorrowerFee,setFeeReceiver.
As such, the contract ownership plays a critical role in the protocol. This critical ownership transfer in one-step is very risky because it is irrecoverable from any mistakes.

Impact

If an incorrect address, e.g. for which the private key is not known, is used accidentally then it prevents the use of all the onlyOwner() functions forever, which includes the changing of various critical addresses and parameters. This use of incorrect address may not even be immediately apparent given that these functions are probably not used immediately. When noticed, due to a failing onlyOwner() function call, it will force the redeployment of these contracts and require appropriate changes and notifications for switching from the old to new address. This will diminish trust in the protocol and incur a significant reputational damage.

Proof of Concept

See similar High Risk severity finding from Trail-of-Bits Audit of Hermez: https://github.com/trailofbits/publications/blob/master/reviews/hermez.pdf

See similar Medium Risk severity finding from Trail-of-Bits Audit of Uniswap V3: https://github.com/Uniswap/uniswap-v3-core/blob/main/audits/tob/audit.pdf

Tools Used

Manual review

Recommendations

Use openzeppline's standard Ownable2Step for the ownership transfer that allows some margin of error and easy revert as compared to implemented Ownable.sol's irrecoverable mechanism.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.