Spectra - wangxx2026's results

Lines of code

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

Vulnerability details

Impact

Incompatibility with eip-5095 will result in the integrator not being able to redeem or withdraw funds in accordance with the standard integration.

Proof of Concept

In the documentation, we can see that redeem and withdraw are compliant with the eip-5095 standard. In the redeem and withdraw methods, the standard requires: https://eips.ethereum.org/EIPS/eip-5095#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. MAY support an additional flow in which the principal tokens are transferred to the Principal Token contract before the redeem execution, and are accounted for during redeem.

In the code we can see that if the owner is not equal to msg.sender it REVERTS. https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L829-L831

        if (_owner != msg.sender) {
            revert UnauthorizedCaller();
        }

And according to the standard requirements, when the owner is not equal to msg.sender, it should be performed

_spendAllowance(owner, msg.sender, value); 

in order to satisfy the standard requirement

You can refer to the reference implementation provided by the standard

Tools Used

Manual Review

Recommended Mitigation Steps

1._beforeRedeem can be modified to: if (_owner != msg.sender) { _spendAllowance(_owner, msg.sender, _shares); } 2._beforeWithdraw remove _owner is not equal to msg.sender judgment, add to the _withdrawShares method.

Assessed type

Other

[L-01]The maxWithdraw method does not meet the eip-5095 standard.

eip-5095 requires protocol pause to return 0, the implementation is revert.This can lead to unexpected situations when integrators integrate according to the eip-5095 standard. It is recommended that the eip-5095 standard be implemented so that exceptions return 0 instead of revert.

https://eips.ethereum.org/EIPS/eip-5095#maxwithdraw

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

MUST NOT revert.

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

    function maxWithdraw(address owner) public view override whenNotPaused returns (uint256) {
        return convertToUnderlying(_maxBurnable(owner));
    }

whenNotPaused implementation: https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/67b51f0e69a558a3ce3997c74905d49741b1f504/contracts/utils/PausableUpgradeable.sol#L72-L75

    modifier whenNotPaused() {
        _requireNotPaused();
        _;
    }

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

And convertToUnderlying() will also revert, see [L-02].

[L_02] previewRedeem、convertToPrincipal、convertToUnderlying can be revert does not comply with eip-5095.

eip-5095 standard unless overflow, can not revert. This will lead to the integrator in accordance with the eip-5095 standard integration, the emergence of unexpected situations. It is recommended to implement according to eip-5095 standard, exceptions return 0 instead of revert. eip-5095 requirements: https://eips.ethereum.org/EIPS/eip-5095#converttounderlying

MUST NOT revert unless due to integer overflow caused by an unreasonably large input.

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

    function convertToUnderlying(uint256 principalAmount) public view override returns (uint256) {
        return IERC4626(ibt).previewRedeem(_convertSharesToIBTs(principalAmount, false));
    }

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

    function _convertSharesToIBTs(
        uint256 _shares,
        bool _roundUp
    ) internal view returns (uint256 ibts) {
        (uint256 _ptRate, uint256 _ibtRate) = _getPTandIBTRates(false);
        if (_ibtRate == 0) {
            revert RateError(); // @audit will revert
        }
        ibts = _shares.mulDiv(
            _ptRate,
            _ibtRate,
            _roundUp ? Math.Rounding.Ceil : Math.Rounding.Floor
        );
    }

[L-03] The time at which redeem can be executed does not comply with eip-5095.

eip-5095 requires that the underlying assets cannot be extracted using redeem before expiration. It is recommended to use withdraw method to withdraw the underlying assets before the expiration date. withdraw has no limitation on the usage time.

eip-5095 requirements: https://eips.ethereum.org/EIPS/eip-5095#redeem

At or after maturity, burns exactly principalAmount of Principal Tokens from from and sends underlyingAmount of underlying tokens to to

Code implementation has no expiration time limit: https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L229-L237

    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public override returns (uint256 assets) {
        _beforeRedeem(shares, owner);
        assets = IERC4626(ibt).redeem(_convertSharesToIBTs(shares, false), receiver, address(this));
        emit Redeem(owner, receiver, shares);
    }