Summary
ERC4626 implements a maxRedeem() function that must specify the max amount of shares a user can redeem. Due to the locked shares property, a user can't withdraw all their shares at any time, but the maxRedeem() still returns their balance (inherited).
Vulnerability Detail
When we look at sYUSD::maxWithdraw(), it correctly accounts for the locked shares:
function maxWithdraw(address owner) public view virtual override returns (uint256) {
// Calculate current unlocked amount (without state changes)
uint256 currentUnlocked = unlockedShares[owner];
LockedShares[] storage userLocks = userLockedShares[owner];
for (uint256 i = 0; i < userLocks.length; i++) {
if (block.timestamp >= userLocks[i].expiryTimestamp) {
currentUnlocked += userLocks[i].amount;
}
}
// Convert unlocked shares to assets
return convertToAssets(currentUnlocked);
}However, this is not true for maxRedeem(), which returns the inherited unchanged balanceOf(user) from ERC4626.
Impact
Specification mismatch.
Code Snippet
Tool Used
Manual Review
Recommendation
Implement maxRedeem(), for example:
function maxRedeem(address owner) public view virtual override returns (uint256) {
// Calculate current unlocked amount (without state changes)
uint256 currentUnlocked = unlockedShares[owner];
LockedShares[] storage userLocks = userLockedShares[owner];
for (uint256 i = 0; i < userLocks.length; i++) {
if (block.timestamp >= userLocks[i].expiryTimestamp) {
currentUnlocked += userLocks[i].amount;
}
}
return currentUnlocked;
}