Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of guarded launch approach may be risky #76

Open
code423n4 opened this issue Sep 29, 2021 · 2 comments
Open

Lack of guarded launch approach may be risky #76

code423n4 opened this issue Sep 29, 2021 · 2 comments
Labels

Comments

@code423n4
Copy link
Contributor

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.

@code423n4 code423n4 added 1 (Low Risk) bug Something isn't working labels Sep 29, 2021
code423n4 added a commit that referenced this issue Sep 29, 2021
@talegift
Copy link
Collaborator

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
Copy link
Collaborator

Best practices, non-critical

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants