Spectra - Tigerfrake's results

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L335

Vulnerability details

Impact

The redeem() function in the context of an ERC4626 token, as mentioned in the OpenZeppelin documentation, is designed to allow token holders to redeem their shares for underlying assets. According to the specification, the redeem() function:

MUST revert if all of shares cannot be redeemed (due to withdrawal limit being reached, slippage, the owner not having enough shares, etc).

However, in the claimFees() function, the line assets = IERC4626(ibt).redeem(ibts, msg.sender, address(this)); does not explicitly include this check and may result in loss of funds for the user.

Proof of Concept

• https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L329-L337

    function claimFees() external override returns (uint256 assets) {
        if (msg.sender != IRegistry(registry).getFeeCollector()) {
            revert UnauthorizedCaller();
        }
        uint256 ibts = unclaimedFeesInIBT;
        unclaimedFeesInIBT = 0;
        assets = IERC4626(ibt).redeem(ibts, msg.sender, address(this));
        emit FeeClaimed(msg.sender, ibts, assets);
    }

The worst possible scenario in the context of the redeem() function in ERC4626 tokens, considering the discrepancy between the expected behavior outlined in the OpenZeppelin documentation and the actual implementation observed in the claimFees() function, could involve a failure to properly handle cases such as:

• Slippage: Slippage occurs when the price of an asset changes between the time of a transaction and the time it is executed. If the redeem() function does not revert when slippage is too high, users might end up with less value in their redemption than they expected. This is because the price of the underlying assets could have decreased between the time the redemption request was made and when it was processed, leading to a loss of value.

Tools Used

Manual Review

Recommended Mitigation Steps

To ensure that the contract behaves as expected and adheres to the guidelines provided by OpenZeppelin, you might consider adding explicit checks or conditions before calling the redeem() function.

Assessed type

ERC4626