Beedle - Oracle free perpetual lending

Summary

1. In transferOwnership there’s a sanity check to see that the address argument is non-zero, which can fail silently. Consider throwing if the check doesn’t pass, by changing the line to require (newOwner != 0x0);.

2. The Lender contract allows owner to feeReceiver. As such, the contract ownership plays a critical role in the protocol.

Vulnerability Details

Scenario: 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.

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

Impact

See vulnerability details.

Tools Used

Manual Review

Recommendations

Add zero address check in transferOwnership() function in Ownable contract.

Consider two-step address change: 1) Approve a new address as a pendingOwner 2) A transaction from the pendingOwner address claims the pending ownership change. This mitigates risk because if an incorrect address is used in step (1) then it can be fixed by re-approving the correct address. Only after a correct address is used in step (1) can step (2) happen and complete the address/ownership change.

Also, consider adding a time-delay for such sensitive actions. And at a minimum, use a multisig owner address and not an EOA.