search

code-423n4 / 2022-01-elasticswap-findings

1 stars 0 forks source link

calculateQuoteTokenQty() Does Not Check Rebase Event May Cause MisPricing #86

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Meta0xNull

Vulnerability details

Impact

// x*y=k - we track these internally to compare to actual balances of the ERC20's

When Rebase Event Happen, the curve change and affect Pricing for Both Buy & Sell.

calculateBaseTokenQty() Do Check if Experience Quote Token Decay / a Rebase Down event and Adjust Curve, But calculateQuoteTokenQty() Does Not Check. Thus calculateQuoteTokenQty() may have a Wrong Curve after Rebase Event.

Proof of Concept

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/libraries/MathLib.sol#L611-L637 https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/libraries/MathLib.sol#L657-L681

Tools Used

Manual Review

Recommended Mitigation Steps

Check Rebase Event for calculateQuoteTokenQty() and Adjust Curve if Needed.

0xean commented 2 years ago

Incorrect, the AMM doesn't adjust pricing on a rebase event, traders do that through arbitrage and this is by design.

GalloDaSballo commented 2 years ago

I will definitely need to look into this for longer, but @0xean it seems like the system is only checking for a negative rebase of the baseToken exclusively when calling swapQuoteTokenForBaseToken.

I'm guessing the negative-rebase check is there to ensure there's enough liquidity for the swap (while using internal balances for the pricing / ratio), while on a positive rebase you wouldn't care and instead you'd use the internal balances again for pricing?

If that's the case, I'm guessing the "worst case scenario" is that you'd have some dust due to rebalances left if traders exclusively bought and sold, while the liquidity provider would actually have a way to arb out those differences.

Am I understanding the system correctly?

GalloDaSballo commented 2 years ago

Per the information that I have, available here: https://github.com/ElasticSwap/elasticswap/blob/develop/ElasticSwapMath.md

The math is used to swap from baseToken (rebasing or not) to quoteToken (never rebasing) As such, while there may have been a rebase (which can be arbed away through single sided liquidity provision or by doing the opposite swap), due to the AMM guarantees, all the math can be performed via the storage variables.

Considering:

Am going to mark the finding invalid