sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

JohnSmith - `preview*` functions do not follow EIP5095 #184

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

JohnSmith

medium

preview* functions do not follow EIP5095

Summary

previewDeposit, previewMint, previewRedeem, previewWithdraw are not aware of slippage for swaps made to obtain tokens.

Vulnerability Detail

Same issue for all of them, we take a look at previewRedeem in particular.

src/tokens/ERC5095.sol
128:     function previewRedeem(uint256 s) public view override returns (uint256) {
129:         if (block.timestamp > maturity) {
130:             return s;
131:         }
132:         return IYield(pool).sellFYTokenPreview(Cast.u128(s));
133:     }

when we call this at block.timestamp <= maturity we get IYield(pool).sellFYTokenPreview(Cast.u128(s)) called. however when we call redeem we include 1% slippage i.e. on L298

src/tokens/ERC5095.sol
294:                 uint128 returned = IMarketPlace(marketplace).sellPrincipalToken(
295:                     underlying,
296:                     maturity,
297:                     Cast.u128(s),
298:                     assets - (assets / 100)
299:                 );

as result redeem may return up 1% lower amount than stated by previewRedeem

according to EIP5095 https://eips.ethereum.org/EIPS/eip-5095 previewRedeem

MUST return as close to and no more than the exact amount of underliyng that would be obtained in a redeem call in the same transaction. I.e. redeem should return the same or more underlyingAmount as previewRedeem if called in the same transaction.

Impact

Users expect implementation to follow EIP5095 standard, current implementation may lead to some users automation scripts failing and revert transactions and lose value.

Code Snippet

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

Tool used

Manual Review

Recommendation

Include possible slpippage for return value, i.e. for previewRedeem

src/tokens/ERC5095.sol
128:     function previewRedeem(uint256 s) public view override returns (uint256) {//@audit is same as redeem value?
129:         if (block.timestamp > maturity) {
130:             return s;
131:         }
+       uint256 expected = IYield(pool).sellFYTokenPreview(Cast.u128(s));
- 132:         return IYield(pool).sellFYTokenPreview(Cast.u128(s));
+ 132:         return expected - expected / 100;
133:     }