`Principal::deposit` @ line 171 invokes the function without slippage protection

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

The external function Principal::deposit @ line 171 invokes a function with the same name deposit() @ line 176 which doesn't have the slippage protection. See code blocks below.

deposit() @ line 171

function deposit(uint256 assets, address receiver) external override returns (uint256 shares) {
    shares = deposit(assets, receiver, receiver); // @audit invokes desposit() @ line 176
}

deposit() @ line 176

function deposit(
    uint256 assets,
    address ptReceiver,
    address ytReceiver
) public override returns (uint256 shares) {
    IERC20(_asset).safeTransferFrom(msg.sender, address(this), assets);
    IERC20(_asset).safeIncreaseAllowance(ibt, assets);
    uint256 ibts = IERC4626(ibt).deposit(assets, address(this));
    shares = _depositIBT(ibts, ptReceiver, ytReceiver);
}

If the user calls deposit() @ line 171, he/she is susceptible to inconceivable shares loss because of the lack of slippage protection. However, there's another function with the same name that also invokes deposit() @ line 176 with a slippage protection. It's the deposit function deposit() @ line 188, shown below.

function deposit(
    uint256 assets,
    address ptReceiver,
    address ytReceiver,
    uint256 minShares
) external override returns (uint256 shares) {
    shares = deposit(assets, ptReceiver, ytReceiver);
    if (shares < minShares) {   // @audit has slippage protection
        revert ERC5143SlippageProtectionFailed();
    }
}

This is the function that should have been invoked.

Tools Used

Manual Review

Recommended Mitigation Steps

Invoke the deposit() @ line 188 within deposit() @ line 171 by changing the code as shown below.

-- function deposit(uint256 assets, address receiver) external override returns (uint256 shares) {
++ function deposit(uint256 assets, address receiver, uint256 minShares) external override returns (uint256 shares) {

--     shares = deposit(assets, receiver, receiver);
++     shares = deposit(assets, receiver, receiver, minShares);
}

Assessed type

Other

[c4-pre-sort]

gzeon-c4 marked the issue as insufficient quality report

[gzeon-c4]

user error

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #246

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid