code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

Lack of guarded launch approach may be risky #76

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The protocol allows listing/trading of arbitrary tokens without an initial time-bounded whitelist of assets or global pause/unpause functionality. This is a risky design because if there are latent protocol vulnerabilities there is no fallback option.

Proof of Concept

While there are functions to set limits on depositing/borrowing and enable/disable deposits/borrowing, there is a lack of asset whitelisting and global pause/unpause functionality in the protocol. Also, it is not clear to what extent the deposit/borrow limits will actually be enforced.

See https://medium.com/electric-capital/derisking-defi-guarded-launches-2600ce730e0a

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider an initial guarded launch approach to owner-based whitelisting of asset/token types and adding a global pause/unpause functionality (e.g. based on OpenZeppelin’s Pausable.sol) for emergency handling. The design should be owner configurable where the owner can renounce ownership at a later point when the protocol operation is sufficiently time-tested and deemed stable/safe.

talegift commented 3 years ago

We actually have plenty of functions to facilitate a guarded launch. I believe we have more of them than many DeFi projects in general.

As the issue mentions correctly, there are several functions for both global and per-pair suspension of functionality.

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L711-L733

Adding an even more specific pause function would result in a higher gas cost for each call. I believe these functions already allow us to sufficiently limit the functionality and capital in the protocol.

A whitelist of tokens would be against the permissionless goal of the protocol. It would be the same as if Uniswap had a whitelist of which tokens you're allowed to trade. Each pair is isolated which moves the decision of which pairs & tokens to use to the user.

ghoul-sol commented 3 years ago

Best practices, non-critical