c4-bot-6 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

In PrincipalToken::redeem @ line 229 users are allowed to redeem assets without slippage protection because the function is exposed public as shown below.

function redeem(
    uint256 shares,
    address receiver,
    address owner
) public override returns (uint256 assets) { // @audit set as "public", should just be "internal"
    _beforeRedeem(shares, owner);
    assets = IERC4626(ibt).redeem(_convertSharesToIBTs(shares, false), receiver, address(this));
    emit Redeem(owner, receiver, shares);
}

It was invoked on another function with the same name PrincipalToken::redeem @ line 240 but this time, it has slippage protection as shown below.

function redeem(
    uint256 shares,
    address receiver,
    address owner,
    uint256 minAssets
) external override returns (uint256 assets) {
    assets = redeem(shares, receiver, owner); // @audit redeem() @ line 229 was invoked here
    if (assets < minAssets) {
        revert ERC5143SlippageProtectionFailed();
    }
}

If the user calls the function PrincipalToken::redeem @ line 229, he/she is susceptible to inconceivable assets loss. The user must call PrincipalToken::redeem @ line 240 because it has the "slippage protection" and it should be explicitly and intentionally enforced.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the function PrincipalToken::redeem @ line 229 from public to internal as shown below.

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

This will prevent the users from calling this function and forces them to use PrincipalToken::redeem @ line 240 which has the slippage protection.

Assessed type

Other

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #246

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid

code-423n4 / 2024-02-spectra-findings