sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

roguereddwarf - MultiInvoker.sol: Unwrap functionality fails when Reserve price drops below one #10

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

roguereddwarf

medium

MultiInvoker.sol: Unwrap functionality fails when Reserve price drops below one

Summary

There are several actions in the MultiInvoker that rely on the _handleUnwrap function which makes use of the DSU Reserve.

The issue is that when the DSU price in the Reserve drops below one, all the upstream functions relying on _handleUnwrap will fail which leads to a DOS of the MultiInvoker

Vulnerability Detail

If the batcher is not set or does not have sufficient balance, reserve.redeem is called in order to redeem DSU for USDC:

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L368-L377

Based on the source code of the reserve contract (https://etherscan.io/address/0x363af3acffed0b7181c2e3c56c00922e142100a8#code), the amount of USDC returned is calculated using the redeemPrice():

function redeem(uint256 amount) external nonReentrant notPaused {
    uint256 redeemAmount = _toUsdcAmount(redeemPrice().mul(amount).asUint256());

    _transferFrom(registry().dollar(), msg.sender, address(this), amount);
    _burnDollar(amount);
    _redeemVault(redeemAmount);
    _transfer(registry().usdc(), msg.sender, redeemAmount);

    emit Redeem(msg.sender, amount, redeemAmount);
}

Usually this price is one but it can drop below one:

function redeemPrice() public view returns (Decimal.D256 memory) {
    return Decimal.min(reserveRatio(), Decimal.one());
}

When the redeem price drops below one is when the issue occurs because MultiInvoker._handleUnwrap will try to transfer USDC to the receiver at a 1-to-1 exchange rate (which will fail due to the insufficient USDC balance).

Impact

The components integrating with the MultiInvoker will experience a DOS when the redeemPrice() drops below one.

Code Snippet

Inspect the source code of the DSU Reserve here: https://etherscan.io/address/0x363af3acffed0b7181c2e3c56c00922e142100a8#code

Code from MultiInvoker._handleUnwrap: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L368-L377

Tool used

Manual Review

Recommendation

The _handleUnwrap function should query the DSU - USDC exchange rate from the Reserve contract

function redeemPrice() public view returns (Decimal.D256 memory) {
    return Decimal.min(reserveRatio(), Decimal.one());
}

Based on this exchange rate, it needs to be calculated how much USDC will be received from the Reserve and this adjusted amount then needs to be pushed to the receiver.

KenzoAgada commented 1 year ago

Valid finding, but does not break core protocol functionality nor causes loss of user funds. Considering as a valid low.