MsgSwapOrder will never work for Canto nodes

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-canto/blob/d1d51b2293d4689f467b8b1c82bba84f8f7ea008/canto-main/proto/canto/coinswap/v1/tx.proto#L96-L104

Vulnerability details

Impact

MsgSwapOrder will never work for Canto nodes

Proof of Concept

An oversight in the MsgSwapOrder where the tag is directed to nested Input message lacks the necessary cosmos.msg.v1.signer to indirectly identify the signer

message Input {
  string address = 1;
  cosmos.base.v1beta1.Coin coin = 2 [ (gogoproto.nullable) = false ];
}

Tools Used

Eyes

Recommended Mitigation Steps

Add DefineCustomGetSigners call in app.go for the coinswap Input message like u did for MsgConvertERC20

https://github.com/code-423n4/2024-05-canto/blob/d1d51b2293d4689f467b8b1c82bba84f8f7ea008/canto-main/app/app.go#L316

signingOptions.DefineCustomGetSigners(protov2.MessageName(&erc20v1.MsgConvertERC20{}), erc20types.GetSignersFromMsgConvertERC20V2)

Assessed type

Other

[C4-Staff]

CloudEllie marked the issue as not a duplicate

[C4-Staff]

CloudEllie marked the issue as primary issue

[poorphd]

• Label
  • sponsor confirmed
• Reasoning
  • The liquidity pools used for onboarding are directly called by the keeper method in the IBC middleware, so there is no problem with the onboarding function because the swap occurs. However, since only the swap from the ibc voucher to canto takes place, if there is a price discrepancy, a mechanism is needed for the arbitrager to return to the appropriate price through MsgSwapOrder.
• Severity
  • Mid → QA
  • In order to abuse this for price manipulation, it is necessary to repeatedly make auto-swaps through IBC transfers, but the auto-swap function only works when the balance of canto is less than 4, making it difficult to manipulate prices. This issue is valid, but since it is not an issue that opposes the real risk of assets, the severity should be adjusted from Mid to QA.
• Patch
  • We will patch this before the v0.50 production release

[3docSec]

As the sponsor said, the effect of this vulnerability is that the pools' price drifts won't be balanced by a necessary arbitraging force which is required for the swap to meet the slippage / maxSwapAmount check, hence impacting the availability of the Onboarding functionality. For this reason, I find M an appropriate severity for this finding.

[c4-judge]

3docSec marked the issue as satisfactory

[c4-judge]

3docSec marked the issue as selected for report

[c4-sponsor]

@poorphd Sponsors are not allowed to close, reopen, or assign issues or pull requests.