search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

`withdrawAll()` has a rug risk in `LiquidityPool.sol` #144

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/pool/LiquidityPool.sol#L126

Vulnerability details

Impact

Although withdrawAll() is supposed to be called in case of emergencies by governors, there is still a significant "rug risk". Governors can withdraw all funds from the vault unconditionally.

Proof of Concept

    /**
     * @notice Withdraws all funds from vault.
     * @dev Should be called in case of emergencies.
     */
    function withdrawAll() external override onlyGovernance {
        getVault().withdrawAll();
    }

Tools Used

vim

Recommended Mitigation Steps

Add a timelock to limit governors.

chase-manning commented 2 years ago

This is an emergency function that withdraws all funds from the Vault and Strategy and shuts down the strategy. The funds are withdrawn to the Pool and so there is no way for the Governance multisig to drain any funds. If the multisig calls this function users can safely withdraw their funds from the Pool. It is the expected use case of the function. There is no "rug risk".