IERC4626::redeem() not according to spec and may result in loss of funds #23
Description
Lines of code
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
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 thepriceof theunderlying assetscould havedecreasedbetween the time the redemption request was made and when it was processed, leading to aloss 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