code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

Dependency on `whenNotPaused` Modifier in Withdraw and Redeem Functions #57

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L805

Vulnerability details

Impact

The protocol allows users to be able to redeem shares only when the protocol is not paused.

Users may face difficulties withdrawing or redeeming their assets during critical periods if the contract is paused. This limitation can hinder users' ability to respond promptly to changing market conditions or unexpected events, potentially resulting in financial losses or frustration.

Proof of Concept

 function redeem(
        uint256 shares, 
        address receiver,
        address owner
    ) public override returns (uint256 assets) {
        _beforeRedeem(shares, owner);
        assets = IERC4626(ibt).redeem(_convertSharesToIBTs(shares, false), receiver, address(this));
        emit Redeem(owner, receiver, shares);
    }

the redeem function calls the beforeRedeem which is shown below

/**
     * @dev Internal function for preparing redeems
     * @param _shares The amount of shares being redeemed
     * @param _owner The address of shares' owner
     */
    function _beforeRedeem(uint256 _shares, address _owner) internal nonReentrant whenNotPaused {
        if (_owner != msg.sender) {
            revert UnauthorizedCaller();
        }
        if (_shares > _maxBurnable(_owner)) {
            revert UnsufficientBalance();
        }
        if (block.timestamp >= expiry) {
            if (!ratesAtExpiryStored) {
                storeRatesAtExpiry();
            }
        } else {
            updateYield(_owner);
            IYieldToken(yt).burnWithoutUpdate(_owner, _shares);
        }
        _burn(_owner, _shares);
    }

The withdraw and redeem functions in the protocol are currently utilizing the whenNotPaused modifier. While this modifier serves to prevent the execution of functions when the contract is in a paused state, its usage in critical functions like withdraw and redeem poses several limitations and potential risks.

Tools Used

manual review

Recommended Mitigation Steps

reconsider the usage of the whenNotPaused modifier in critical functions like withdraw and redeem. Instead, the protocol should explore alternative approaches to ensure user accessibility, security, and robustness. Possible alternatives include implementing emergency withdrawal mechanisms,

Assessed type

DoS

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 8 months ago

intended behavior

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid