Protocol not compliant to EIP 5095

[c4-bot-2]

Lines of code

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

Vulnerability details

Impact

The Spectra protocol claims that

The Principal Token is EIP-5095 and EIP-2612 compliant.

Other protocols that integrate with Spectra may wrongly assume that the functions are EIP-5095 compliant. Thus, it might cause integration problems in the future that can lead to wide range of issues for both parties.

Proof of Concept

Let's go through the EIP-5095 standard and look how it expected to be implemented

maxRedeem()/maxWithdraw() not compliant to EIP 5095

functions maxRedeem() and maxWithdraw()

MUST factor in both global and user-specific limits, like if redemption is entirely disabled (even temporarily) it MUST return 0.

Current implementation of maxRedeem() ignores paused state, because does not have modifier whenNotPaused must return 0 when paused

MUST NOT revert.

Current implementation of maxWithdraw() reverts when paused, must return 0.

previewRedeem()/previewWithdraw() not compliant to EIP-5095

function previewRedeem()

MUST return as close to and no more than the exact amount of underliyng that would be obtained in a redeem call in the same transaction. I.e. redeem should return the same or more underlyingAmount as previewRedeem if called in the same transaction. MUST be inclusive of redemption fees. Integrators should be aware of the existence of redemption fees. previewRedeem() current implementation does not include redemption fees.

function previewWithdraw()

MUST return as close to and no fewer than the exact amount of principal tokens that would be burned in a withdraw call in the same transaction. I.e. withdraw should return the same or fewer principalAmount as previewWithdraw if called in the same transaction. MUST be inclusive of withdrawal fees. Integrators should be aware of the existence of withdrawal fees. /previewWithdraw() current implementation does not include withdrawal fees.

redeem()/withdraw() not compliant to EIP-5095

function redeem()

MUST support a redeem flow where the Principal Tokens are burned from holder directly where holder is msg.sender or msg.sender has EIP-20 approval over the principal tokens of holder. function withdraw() MUST support a withdraw flow where the principal tokens are burned from holder directly where holder is msg.sender or msg.sender has EIP-20 approval over the principal tokens of holder.

In current implementation both functions support only case when msg.sender is holder, by standard we MUST support also case when msg.sender has EIP-20 approval over the principal tokens of holder.

    function _beforeRedeem(uint256 _shares, address _owner) internal nonReentrant whenNotPaused {
        if (_owner != msg.sender) {
            revert UnauthorizedCaller();
        }
    ...

    function _beforeWithdraw(uint256 _assets, address _owner) internal whenNotPaused nonReentrant {
    if (_owner != msg.sender) {
        revert UnauthorizedCaller();
    }
    ...

redeem or withdraw before maturity is not compliant to EIP-5095

by standard

maturity: The timestamp (unix) at which a Principal Token matures. Principal Tokens become redeemable for underlying at or after this timestamp. Current implementation of redeem and withdraw allow redeemal before maturity date.

Implementing ERC-5095 in a way that allows tokens to be redeemed not only after but also before the expiry date would not be compliant with the ERC-5095 standard as described in the provided reference implementation. ERC-5095 is designed for principal tokens, which are redeemable for a single underlying EIP-20 token at a future timestamp specified by the maturity parameter. The reference implementation includes a modifier afterMaturity that ensures operations related to redeeming tokens can only be executed if the current block timestamp is at or after the maturity date. This modifier is applied to the redeem and withdraw functions, enforcing that these operations can only occur after the maturity date.

The key requirement for compliance with ERC-5095 is that redemption can only occur after the maturity date, as per the standard's design and the provided reference implementation. Modifying this behavior to allow redemption before the expiry date would contradict the specification's intent and the expected behavior of principal tokens. Therefore, any implementation that deviates from this core requirement would not be considered compliant with ERC-5095.

Recommended Mitigation Steps

Go through EIP-5095 standard and follow it for all methods

Assessed type

Other

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #33

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-judge]

JustDravee marked the issue as satisfactory