c4-bot-9 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

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

depositIBT @ line 201

function depositIBT(uint256 ibts, address receiver) external override returns (uint256 shares) {
    shares = depositIBT(ibts, receiver, receiver); // @audit invokes depositIBT() @ line 206
}

depositIBT() @ line 206

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

If the user calls depositIBT() @ line 201, 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 depositIBT() @ line 206 and this time, with a slippage protection. It's the deposit function depositIBT() @ line 216, shown below.

depostIBT() @ line 216

function depositIBT(
    uint256 ibts,
    address ptReceiver,
    address ytReceiver,
    uint256 minShares
) external override returns (uint256 shares) {
    shares = depositIBT(ibts, ptReceiver, ytReceiver); // @audit invokes depostiIBT() @ line 206
    if (shares < minShares) {
        revert ERC5143SlippageProtectionFailed();
    }
}

This is the function that should have been invoked.

Tools Used

Manual Review

Recommended Mitigation Steps

Invoke the depostIBT() @ line 216 within depositIBT @ line 201 and change the code as shown below.

-- function depositIBT(uint256 ibts, address receiver) external override returns (uint256 shares) {
++ function depositIBT(uint256 ibts, address receiver, uint256 minShares) external override returns (uint256 shares) {

--    shares = depositIBT(ibts, receiver, receiver);
++    shares = depositIBT(ibts, receiver, receiver, minShares);
}

Assessed type

Other

c4-pre-sort commented 6 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 6 months ago

user error

c4-pre-sort commented 6 months ago

gzeon-c4 marked the issue as primary issue

c4-judge commented 6 months ago

JustDravee marked the issue as unsatisfactory: Invalid

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