Lack of slippage controls in PrincipalToken::deposit and PrincipalToken::withdraw

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L176 https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L278

Vulnerability details

Impact

The deposit and withdraw functions do not check that users receive the expected number of shares based on previewed rates. This can lead to slippage and loss of value.

Proof of Concept

In the deposit function, shares are minted based on current rates:

File: PrincipalToken.sol
176:     function deposit(
177:         uint256 assets,
178:         address ptReceiver,
179:         address ytReceiver
180:     ) public override returns (uint256 shares) {
181:         IERC20(_asset).safeTransferFrom(msg.sender, address(this), assets);
182:         IERC20(_asset).safeIncreaseAllowance(ibt, assets);
183:         uint256 ibts = IERC4626(ibt).deposit(assets, address(this));
184:         shares = _depositIBT(ibts, ptReceiver, ytReceiver);
185:     }

Line of Code

There is no check that shares exceeds a minimum amount expected by the user.

Similarly in withdraw:

File: PrincipalToken.sol
278:     function withdraw(
279:         uint256 assets,
280:         address receiver,
281:         address owner
282:     ) public override returns (uint256 shares) {
283:         _beforeWithdraw(assets, owner);
284:         (uint256 _ptRate, uint256 _ibtRate) = _getPTandIBTRates(false);
285:         uint256 ibts = IERC4626(ibt).withdraw(assets, receiver, address(this));
286:         shares = _withdrawShares(ibts, receiver, owner, _ptRate, _ibtRate);
287:     }

Line of Code

Scenario

1. Attacker manipulates market to change IBT/PT rates unfavorably

2. User calls previewDeposit and expects to receive 100 shares for 10 assets

3. Attacker stops manipulation, allowing rates to return to normal

4. User deposits 10 assets, but only receives 90 shares due to updated rates

5. User lost value due to slippage between preview and execution

Similar for withdraw - user may receive fewer assets than expected.

Tools

Manual Review

Recommended Mitigation Steps

• Require minimum share amounts on deposit. Revert if received is less.

• Require maximum share amounts on withdraw. Revert if received is more.

Assessed type

Invalid Validation

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-pre-sort]

gzeon-c4 marked the issue as primary issue

[c4-pre-sort]

gzeon-c4 marked the issue as insufficient quality report

[gzeon-c4]

there is an overload that allow slippage control

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #246

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid