search

FraxFinance / dev-fraxswap

Fraxswap for Fraxtal
0 stars 0 forks source link

bug[high]: No validations on `tokenOut` params of `FraxswapRoute`, `FraxswapParams` #10

Open pegahcarter opened 5 months ago

pegahcarter commented 5 months ago

Description

When swapping with the Multihop Router, a user passes in their FraxswapParams struct, which gets decoded and referenced for swap routes. However, these routes are not validated and can be incorrectly called. For example, each FraxswapRoute has the following arguments: https://github.com/FraxFinance/dev-fraxswap/blob/9744c757f7e51c1deee3f5db50d3aaec495aea01/src/contracts/periphery/interfaces/IFraxswapRouterMultihop.sol#L36-L37 At execution, there is no validation to ensure these assumptions hold, meaning a user can break the intended logic and have the Multihop act in unintended ways (by trading intermittent routes that are different than expected).

There is also missing validation that the last FraxswapRoute.tokenOut does not equal FraxswapParams.tokenOut.

Minimum amount out is properly enforced so this should not impact losing funds on a bad route.

pegahcarter commented 5 months ago

Upgrading to "high" as a user can lose funds over incorrectly using the complex FraxswapParams. Assumptions of intended FraxswapParams use should be enforced by the router.

denett commented 5 months ago

Token in, token in amount, token out and min amount out are enforced. The route can be anything, but the transaction will fail when there are not enough tokens coming out.

pegahcarter commented 5 months ago

I agree with you that if amountOut = 0 and the user receives 0, the assertion is correct. However, the minimum of 0 is related to slippage and not user entry error. A user should not receive 0 tokens because they're stuck in the pair/router.