The `calculateWithExactInput` uses the same state's values for all transactions in the block

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/keeper/swap.go#L48-L49

Vulnerability details

Impact

The calculateWithExactInput uses the same state's values for all transactions. So all checks which should regulate swapped amounts will be broken. It can be a case of asset loss if there will be a significant amount of transactions in one block.

Proof of Concept

The onboarding module just modifies amount and types of tokens which users should receives in the canto network but doesn't change the state. So all transactions which are executed by the mudule before state updating checked with the same state values. OnRecvPacket function calls TradeInputForExactOutput. TradeInputForExactOutput calls calculateWithExactInput. calculateWithExactInput receives values from the state in lines 48-49. These values are similar for all transactions in one block.

Tools Used

Manual review

Recommended Mitigation Steps

In such cases user's transactions should be bunched in one transaction to prevent unexpected influence on the state.

Assessed type

Reentrancy

[c4-pre-sort]

JeffCX marked the issue as low quality report

[JeffCX]

The calculateWithExactInput uses the same state's values for all transactions. So all checks which should regulate swapped amounts will be broken. It can be a case of asset loss if there will be a significant amount of transactions in one block

don't think there is reentrancy or concurrency involved, not valid

[tkkwon1998]

agreed with @JeffCX, there is no need to persist every call to state immediately as it doesn't query the clean state to do operations.

[c4-sponsor]

tkkwon1998 marked the issue as sponsor disputed

[c4-judge]

0xean marked the issue as unsatisfactory: Invalid