sherlock-audit / 2024-01-napier-judging

8 stars 5 forks source link

xiaoming90 - Tranche will be DOSed when FRAX stop accepting additional ETH staking #103

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

xiaoming90

medium

Tranche will be DOSed when FRAX stop accepting additional ETH staking

Summary

Tranche will be DOSed when FRAX stops accepting additional ETH staking, leading the deposit function (core feature) to stop working.

Vulnerability Detail

[!NOTE]

The issue does not require FRAX admin to be malicious. The submit feature might be paused for legitimate reasons, such as the contract running out of validators or restricting the amount of ETH staked within the FRAX protocol.

When users deposit into the Tranche, a certain portion of the ETH will be staked into FRAX. The adaptor will call the submit function at Line 74.

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/adapters/frax/SFrxETHAdapter.sol#L72

File: SFrxETHAdapter.sol
71:     /// @notice Mint sfrxETH using WETH
72:     function _stake(uint256 stakeAmount) internal override returns (uint256) {
73:         IWETH9(Constants.WETH).withdraw(stakeAmount);
74:         FRXETH_MINTER.submit{value: stakeAmount}();
75:         uint256 received = STAKED_FRXETH.deposit(stakeAmount, address(this));
76:         if (received == 0) revert InvariantViolation();
77: 
78:         return stakeAmount;
79:     }

However, it is possible that the FRAX admin has stopped accepting new ETH if the submitPaused has been set to True by the admin/governance. As a result, the SFrxETHAdapter._stake function will revert whenever users deposit into the Tranche, which effectively DOS the Tranche.

https://etherscan.io/address/0xbAFA44EFE7901E04E39Dad13167D089C559c1138#code#F1#L85

function _submit(address recipient) internal nonReentrant {
    // Initial pause and value checks
    require(!submitPaused, "Submit is paused");
    require(msg.value != 0, "Cannot submit 0");

    // Give the sender frxETH
    frxETHToken.minter_mint(recipient, msg.value);

    // Track the amount of ETH that we are keeping
    uint256 withheld_amt = 0;
    if (withholdRatio != 0) {
        withheld_amt = (msg.value * withholdRatio) / RATIO_PRECISION;
        currentWithheldETH += withheld_amt;
    }

    emit ETHSubmitted(msg.sender, recipient, msg.value, withheld_amt);
}

Impact

The deposit function stopped working. Deposit is the core feature of the Tranche. Breaks core contract functionality

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/adapters/frax/SFrxETHAdapter.sol#L72

Tool used

Manual Review

Recommendation

The protocol should handle the scenario where FRAX stops accepting additional ETH staking gracefully.

Check whether FRAX has stopped accepting new ETH by polling the frxETHMinter.submitPaused() function. If it is returned True, do not proceed with staking ETH to FRAX to avoid reverting the entire deposit TX unnecessarily.

/// @notice Mint sfrxETH using WETH
function _stake(uint256 stakeAmount) internal override returns (uint256) {
+   if (frxETHMinter.submitPaused()) return 0;
    IWETH9(Constants.WETH).withdraw(stakeAmount);
    FRXETH_MINTER.submit{value: stakeAmount}();
    uint256 received = STAKED_FRXETH.deposit(stakeAmount, address(this));
    if (received == 0) revert InvariantViolation();

    return stakeAmount;
}
sherlock-admin commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. External contracts pausing is acceptable