sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

JohnSmith - Loss of tokens on deposit #183

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

JohnSmith

high

Loss of tokens on deposit

Summary

deposit() will return zero shares when called on block.timestamp == maturity

Vulnerability Detail

deposit() call on block.timestamp == maturity will lead to shares = 0

src/tokens/ERC5095.sol
159:         uint128 shares = Cast.u128(previewDeposit(a));//@audit 0 at maturity
src/tokens/ERC5095.sol
108:     function previewDeposit(uint256 a) public view returns (uint256) {
109:         if (block.timestamp < maturity) {
110:             return IYield(pool).sellBasePreview(Cast.u128(a));
111:         }
112:         return 0;
113:     }

which will lead to slippage being zero on swap call

src/tokens/ERC5095.sol
162:         uint128 returned = IMarketPlace(marketplace).sellUnderlying(
163:             underlying,
164:             maturity,
165:             Cast.u128(a),
166:             shares - (shares / 100)//@audit if shares are 0, then all underlying goes to mev bots
167:         );

Impact

User will lose all their tokens to mev bots The fact that block.timestamp can be manipulated by miner/validator only makes such outcome more likely, when malicious parties are collaborating.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L149-L170

Tool used

Manual Review

Recommendation

Revert on maturity as you intended looking at your comments

src/tokens/ERC5095.sol
145:     /// @notice Before maturity spends `assets` of underlying, and sends `shares` of PTs to `receiver`. Post or at maturity, reverts.
src/tokens/ERC5095.sol
149:     function deposit(address r, uint256 a) external override returns (uint256) {
- 150:         if (block.timestamp > maturity) {
+ 150:         if (block.timestamp >= maturity) {

Duplicate of #14