Beedle - Oracle free perpetual lending

Summary

The contracts lack two-step ownership transfer implementation. The basic validation whether the address is not
a zero address is not performed, and the case when the address receiving the role is inaccessible is also not covered properly.

Vulnerability Details

See summary

Impact

The Lender.sol contract allows owners to set/change important parameters such as Lenders fee, Borrower fee, and fee receiver address. As such, the contract ownership plays a critical role in the protocol.

Such critical address transfer/renouncing in one-step is very risky because it is irrecoverable from any mistakes.

Here's a scenario to illustrate why: Let's say you accidentally use an incorrect address — one for which you don't have the private key. This mistake will permanently prevent you from using any functions restricted to the onlyOwner() designation. This includes important tasks like changing critical addresses and parameters. Initially, the use of the incorrect address may go unnoticed, especially since these functions might not be used right away. However, once it's discovered due to a failed onlyOwner() function call, you'll have to redeploy the contracts and make the necessary changes and notifications to switch to a new address. This process will erode trust in the protocol and result in significant damage to its reputation.

Here is a similar high risk severity finding from Trail of Bits: https://github.com/trailofbits/publications/blob/master/reviews/hermez.pdf

Tools Used

Manual review

Recommendations

It is recommended to implement a two-step ownership transfer where separate functions are used for a two-step address change:

1. First, you approve a new address as a "pendingOwner". This step allows you to set the stage for changing the ownership.

2. Next, a transaction from the "pendingOwner" address is needed to claim the pending ownership change. This step is crucial as it ensures that only the correct address can complete the process.

The benefit of this approach is that it mitigates risk. If an incorrect address is mistakenly used in step (1), you can easily fix it by re-approving the correct address. This flexibility gives you the opportunity to rectify any errors before they become permanent.

Additionally, it's worth considering adding a time-delay for such sensitive actions. By introducing a timer or waiting period, you can provide an extra layer of security and give yourself time to double-check the details before finalizing the process.

Furthermore, it is advisable to use a multisig owner address instead of an Externally Owned Account (EOA) as the minimum requirement. This adds an extra level of protection by involving multiple authorized parties in the ownership process, ensuring better control and security.

Implementing these measures enhances the safety and integrity of the ownership change process.