Error occurring between two coin transfers will lead to loss of funds

[code423n4]

Lines of code

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

Vulnerability details

Impact

If the first transfer succeeds but the second transfer fails, the coins will have been deducted from the sender's account but not added to the recipient's account, resulting in a loss of funds.

Proof of Concept

The swapCoins function consists of two separate SendCoins operations: one from the sender to the reserve pool and another from the reserve pool to the recipient. If any error occurs during the second SendCoins operation, the state of the reserve pool would have already been modified by the first SendCoins operation, resulting in an inconsistent state. The swapCoins function does not implement any transactional mechanism to ensure that both SendCoins operations occur as a single atomic operation. If a failure or error occurs after the first SendCoins operation, there is no rollback mechanism to revert the state changes

Tools Used

Manual

Recommended Mitigation Steps

Use a transactional approach that ensures both transfers are executed atomically. This can be achieved by wrapping the two transfers in a single transaction so that they either both succeed or both fail. If an error occurs during the execution, the transaction can be rolled back to maintain a consistent state. https://github.com/seerether/Canto/blob/b9f9741815caab0ae81875f7bd693ea0b4c924eb/cantoreccommend#L9-L29

Assessed type

Token-Transfer

[c4-pre-sort]

JeffCX marked the issue as primary issue

[c4-pre-sort]

JeffCX marked the issue as low quality report

[c4-pre-sort]

JeffCX marked the issue as duplicate of #80

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean changed the severity to 2 (Med Risk)