Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserializing untrusted cosmos transactions with the legacy amino codec can crash the program #691

Closed
2 of 3 tasks
crodriguezvega opened this issue Jan 7, 2022 · 1 comment · Fixed by #725
Closed
2 of 3 tasks
Assignees
Labels
27-interchain-accounts audit Feedback from implementation audit

Comments

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Jan 7, 2022

This issue was found by Trail of Bits during the audit of ICS27 Interchain Accounts

Problem Definition

The DeserializeCosmosTx function can panic and crash the program if it is used with the
legacy amino codec and a specially crafted input. We found this bug by fuzzing the DeserializeCosmosTx function with the legacy amino and protobuf codecs.

This problem can be reproduced with the test

func (suite *TypesTestSuite) TestDeserializeCosmosTxNotCrashes() {
    cdc := codec.NewLegacyAmino()
    marshaler := codec.NewAminoCodec(cdc)
    types.DeserializeCosmosTx(marshaler, []byte{0x10, 0})
}

which can be added directly to ibc-go/modules/apps/27-interchain-accounts/types/codec_test.go file.

Proposal

Investigate and fix the issue that causes the DeserializeCosmosTx function to panic and crash the program when it is used with the legacy amino codec and called with untrusted input. Alternatively, if the function is not intended to be used with untrusted input, add a documentation string that explains this intention and the risks associated with passing untrusted input to this function.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega crodriguezvega added 27-interchain-accounts audit Feedback from implementation audit labels Jan 7, 2022
@crodriguezvega crodriguezvega added this to the Interchain Accounts milestone Jan 7, 2022
@crodriguezvega crodriguezvega moved this to Backlog in ibc-go Jan 7, 2022
@crodriguezvega
Copy link
Contributor Author

Ensure that amino codec is not used for deserializing Cosmos TX and update documentation.

@crodriguezvega crodriguezvega moved this from Backlog to Todo in ibc-go Jan 10, 2022
@colin-axner colin-axner self-assigned this Jan 13, 2022
@crodriguezvega crodriguezvega moved this from Todo to In progress in ibc-go Jan 13, 2022
Repository owner moved this from In progress to Done in ibc-go Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27-interchain-accounts audit Feedback from implementation audit
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants