c4-bot-4 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/25603ac27c3488423a0739b66e784c01a3db7d75/src/tokens/PrincipalToken.sol#L461-L463

Vulnerability details

Vulnerability explanation

According to the almighty EIP-5095, the maxWithdraw() function "MUST return the maximum amount of underlying tokens that could be redeemed from holder through withdraw and not cause a revert". However instead of this the cheeky maxWithdraw() function in the contract will revert when paused because of the whenNotPaused modifier

Impact

The function reverts when the redemptions are paused, which is not in line with the requirements of the EIP-5095.

Proof of Concept

The maxWithdraw()

    function maxWithdraw(address owner) public view override whenNotPaused returns (uint256) {
        return convertToUnderlying(_maxBurnable(owner)); //@note inclusive of fees
    }

reverts when the DAO decides to paused the contract. If the function is called while paused the function reverts.

    function _requireNotPaused() internal view virtual {
        if (paused()) {
 @>          revert EnforcedPause();
        }
    }

Tools Used

EIP-5095 docs

Recommended Mitigation Steps

If the contract happens to be paused, return 0 instead of revertin'.


   -- function maxWithdraw(address owner) public view override whenNotPaused returns (uint256) {

   ++ function maxWithdraw(address owner) public view override returns (uint256) {
 ++    if(paused()) return 0;
       return convertToUnderlying(_maxBurnable(owner)); //@note inclusive of fees
    }

Assessed type

Other

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #33

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as partial-75