Swapping the stableSwapFactory address by admin via setStableSwap in StableSwapRouter can lead to token losses to swap users

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7ea57c5ea8f35d05f067322f96d31133207a37301cfee8c2b7949612a9fef4fa Severity: high

Description: Description\ When the address of stableSwapFactory is changed, all pending swaps will use the new stableSwapFactory. This can lead to unexpected results for end-users including loss of tokens. Attack Scenario\ Consider this attack path:-

1. User A sends a swap transaction to StableSwapRouter.
2. The transactions sits in the mempool awaiting execution since it was sent with low gas fees.
3. Meanwhile, the project admin intends to migrate to a new stableSwapFactory after a vulnerability is discovered in the current stableSwapFactory contract, and therefore sends his transaction via MEV.
4. The new stableSwapFactory has the necessary set of tokens pools user A intends to use but uses different contracts User A was targetting, therefore the transaction is executed by the new stableSwapFactory contract.

User A therefore gets low tokens.

Attachments

1. Proof of Concept (PoC) File Add this POC to tests/router_with_rose.test.ts:

    it("should change stableSwapFactory", async () => {

        await stableSwapRouter.setStableSwap(stableSwapFactory.address,stableSwapInfo.address) ;
        await stableSwapRouter.exactInputStableSwap([ROSE,stROSE.address], [2],ADD_AMOUNT_1, 0,accounts[0].address,{ value: ADD_AMOUNT_1});

    });

2. Revised Code File (Optional) Consider allowing users to pass in the expected stableSwapFactory address to StableSwapRouter::exactInputStableSwap

[omega-audits]

it is not clear why "user a therefefore gets low tokens"

[Ngugs]

Better wording could be- User A CAN get less tokens, for one reason or another, e.g. lesser liquidity in the new pool compared to the pool A had in mind when swapping.

[omega-audits]

In the scenario you describe, there is a vulnerability discovered in the original swap factory. If that is an exploitable vulnerability, the user will probably be happy that their transaction is routed through the updated factory.

Re getting less tokens: there are many reasons why a user may "get low tokens". There is a slippage protection built in that the user can use to limit those risks.

Sorry, but this is not a valid issue imo.