The EIP-5095 standard states that the function MUST factor in both global and user-specific limits, like if redemption is entirely disabled (even temporarily) it MUST return 0.
When the protocol is paused and redemption is temporarily disabled, this function should return 0.
Yet, both functions returns the value as if redemption is still enabled, which is a clear-cut violation of the EIP-5095 standard.
maxRedeem
Maximum amount of principal tokens that can be redeemed from the holder balance, through a redeem call.
MUST return the maximum amount of principal tokens that could be transferred from holder through redeem and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).
MUST factor in both global and user-specific limits, like if redemption is entirely disabled (even temporarily) it MUST return 0.
MUST NOT revert.
maxWithdraw
Maximum amount of the underlying asset that can be redeemed from the holder principal token balance, through a withdraw call.
MUST return the maximum amount of underlying tokens that could be redeemed from holder through withdraw and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).
MUST factor in both global and user-specific limits, like if withdrawals are entirely disabled (even temporarily) it MUST return 0.
MUST NOT revert.
PrincipalToken.sol
/** @dev See {IPrincipalToken-maxWithdraw}.
*/
function maxWithdraw(address owner) public view override whenNotPaused returns (uint256) {
return convertToUnderlying(_maxBurnable(owner));
}
PrincipalToken.sol
/** @dev See {IPrincipalToken-convertToUnderlying}. */
function convertToUnderlying(uint256 principalAmount) public view override returns (uint256) {
return IERC4626(ibt).previewRedeem(_convertSharesToIBTs(principalAmount, false));
}
PrincipalToken.sol
/** @dev See {IPrincipalToken-maxRedeem}. */
function maxRedeem(address owner) public view override returns (uint256) {
return _maxBurnable(owner);
}
PrincipalToken.sol
/**
* @notice Computes the maximum amount of burnable shares for a user
* @param _user The address of the user
* @return maxBurnable The maximum amount of burnable shares
*/
function _maxBurnable(address _user) internal view returns (uint256 maxBurnable) {
if (block.timestamp >= expiry) {
maxBurnable = balanceOf(_user);
} else {
uint256 ptBalance = balanceOf(_user);
uint256 ytBalance = IYieldToken(yt).balanceOf(_user);
maxBurnable = (ptBalance > ytBalance) ? ytBalance : ptBalance;
}
}
Tools Used
Manual Review
Recommended Mitigation Steps
Change the functions so that it will return 0 when the protocol is paused.
Lines of code
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L482-L485 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L458-L462
Vulnerability details
Impact
The EIP-5095 standard states that the function MUST factor in both global and user-specific limits, like if redemption is entirely disabled (even temporarily) it MUST return 0.
When the protocol is paused and redemption is temporarily disabled, this function should return 0.
Yet, both functions returns the value as if redemption is still enabled, which is a clear-cut violation of the EIP-5095 standard.
Proof of Concept
From: EIP-5095
PrincipalToken.sol
PrincipalToken.sol
PrincipalToken.sol
PrincipalToken.sol
Tools Used
Manual Review
Recommended Mitigation Steps
Change the functions so that it will return 0 when the protocol is paused.
Assessed type
Other