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.

Sign up for GitHub

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

Jump to bottom

ValidateBasic is not called when receiving packets from consumers #634

Closed
2 tasks done
Tracked by #754
mpoke opened this issue Jan 3, 2023 · 2 comments · Fixed by #1460
Closed
2 tasks done
Tracked by #754

ValidateBasic is not called when receiving packets from consumers #634

mpoke opened this issue Jan 3, 2023 · 2 comments · Fixed by #1460
Assignees
@mpoke
Labels
good first issue Good for newcomers S: KTLO Keeping the lights on: Keeping the current product operational (bugs, troubleshooting, deps updates) type: bug Issues that need priority attention -- something isn't working

Comments

@mpoke
Copy link
Contributor

mpoke commented Jan 3, 2023

Problem

When receiving packets from a consumer chain, the provider doesn't validate them (by calling ValidateBasic, for VSCMaturedPacketData and SlashPacketData), i.e.,

interchain-security/x/ccv/provider/ibc_module.go

Line 184 in 0c62722

// TODO: call ValidateBasic method on consumer packet data

Related to #544

Closing criteria

ValidateBasic is called in OnRecvPacket.

TODOs

  • Labels have been added for issue
  • Issue has been added to the ICS project
@mpoke mpoke added the type: bug Issues that need priority attention -- something isn't working label Jan 3, 2023
This was referenced Jan 3, 2023
Closed
Closed
@sainoe
Copy link
Contributor

sainoe commented Jan 5, 2023

I think that #609 should be solved beforehand since it entails renaming the consumer packet type in ccv.proto.

@shaspitz shaspitz mentioned this issue Jan 5, 2023
Merged
2 tasks
@mpoke mpoke added this to the Untrusted consumers milestone Jan 27, 2023
@mpoke mpoke added this to Cosmos Hub Jan 27, 2023
@github-project-automation github-project-automation bot moved this to 🩹 Triage in Cosmos Hub Jan 27, 2023
@mpoke mpoke removed this from the Untrusted consumers milestone Mar 5, 2023
@mpoke mpoke moved this from 🩹 Triage to 📥 Todo in Cosmos Hub Mar 5, 2023
@mpoke mpoke mentioned this issue Mar 6, 2023
Closed
@mpoke mpoke added S: KTLO Keeping the lights on: Keeping the current product operational (bugs, troubleshooting, deps updates) good first issue Good for newcomers labels Sep 13, 2023
@mpoke mpoke mentioned this issue Nov 30, 2023
Merged
21 tasks
@mpoke mpoke moved this from 📥 F2: Todo to 🏗 F3: InProgress in Cosmos Hub Nov 30, 2023
@mpoke mpoke self-assigned this Nov 30, 2023
@mpoke mpoke moved this from 🏗 F3: InProgress to 👀 F3: InReview in Cosmos Hub Nov 30, 2023
@mpoke mpoke linked a pull request Dec 1, 2023 that will close this issue
fix!: improve message validation #1460
Merged
21 tasks
@mpoke
Copy link
Contributor Author

mpoke commented Dec 1, 2023

Fixed by #1460

@mpoke mpoke closed this as completed Dec 1, 2023
@github-project-automation github-project-automation bot moved this from 👀 F3: InReview to 👍 F4: Assessment in Cosmos Hub Dec 1, 2023
@mpoke mpoke moved this from 👍 F4: Assessment to ✅ Done in Cosmos Hub Dec 1, 2023
ThanhNhann pushed a commit to decentrio/interchain-security that referenced this issue Jan 3, 2024
@ramacarlucho @colin-axner
@seantking @dependabot @damiannolan @crodriguezvega @tac0turtle @fedekunze
fix: support custom chain IDs for testing (cosmos#774)
8dfbc9c 
* Backport commits from main to v3 release branch (cosmos#682)

* reorganize channel handshake handler (cosmos#647)

* reorganize channel handshake handler

split out channel state changes into its own function.

* readjust 27-interchain-accounts to not rely on state being set before the application callback

* add changelog and migration doc entry

* Update modules/core/04-channel/keeper/handshake.go

* docs: ICA Overview (cosmos#626)

* docs: ica overview

* fix: ordering

* add spacing

* fix: spacing

* fix: remove bulletpoints

* fix: wording

* update go mod for security vulnerabilities (cosmos#655)

* update go mod for security vulnerabilities

* update package-lock.json

* update vue dependency (cosmos#662)

* bump glob-parent version in json package (cosmos#663)

* build(deps): bump actions/setup-go from 2.1.4 to 2.1.5 (cosmos#656)

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 2.1.4 to 2.1.5.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v2.1.4...v2.1.5)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: colin axnér <[email protected]>

* docs: begin removal of internal "spec" directories (cosmos#634)

* begin removal of spec docs within core ibc

* remove broken link

* Apply suggestions from code review

Co-authored-by: Damian Nolan <[email protected]>

* remove broken link

* remove broken links

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

* Modify `OnChanOpenTry` application callback to perform app version negotitation (cosmos#646)

* remove NegotiateAppVersion and AppVersion gRPC (cosmos#643)

The NegotiateAppVersion callback has been removed from the IBC Application interface.
The gRPC AppVersion has been removed.
The app version negoitation will be handled by applications by returning the version in OnChanOpenTry.

* Modify `OnChanOpenTry` to return application version (cosmos#650)

* modify OnChanOpenTry to return negotiated version

modify IBCModule interface function OnChanOpenTry to return the negotiated app version. Tests have not been updated

* fix ibc_module_test.go tests

* fix tests

* Apply suggestions from code review

* add handshake test case

* add CHANGELOG and migration docs

* update documentation

* fix broken link

* fix broken link (cosmos#664)

* chore: update make build-docs, add docs build checker (cosmos#667)

* update Makefile, add docs build checker

* Update .github/workflows/check-docs.yml

Co-authored-by: Marko <[email protected]>

Co-authored-by: Marko <[email protected]>

* register ICA query server, fix panics in params query cli (cosmos#666)

Register the controller and host query servers to a chain.
Returns an error upon cli params query failure instead of panicing.

* update of roadmap with latest release (cosmos#653)

* update of roadmap with latest release and changed the way release versions are encoded

* fixed typo

Co-authored-by: Carlos Rodriguez <[email protected]>

* build(deps): bump actions/checkout from 2.3.1 to 2.4.0 (cosmos#672)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2.3.1 to 2.4.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2.3.1...v2.4.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* allow ics20 to connect to middleware (cosmos#675)

* allow ics20 to connect to middleware

Creates ics4Wrapper which allows middleware applications to only implement SendPacket to connect ics20 as an application in its middleware stack

* add changelog and migration doc

* fix migration doc spelling

* fix: register InterchainAccount as x/auth GenesisAccount (cosmos#676)

* adding GenesisAccount interface registration for InterchainAccount impl

* updating RegisterInterfaces ica godoc

* enable mergify for backports (cosmos#678)

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Sean King <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

* fixes evmos tests

* cleanup

* changelog and cleanup

* pr suggestions. bump chainid

Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Sean King <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mpoke mpoke

Labels
good first issue Good for newcomers S: KTLO Keeping the lights on: Keeping the current product operational (bugs, troubleshooting, deps updates) type: bug Issues that need priority attention -- something isn't working
Projects
Cosmos Hub
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix!: improve message validation cosmos/interchain-security
2 participants
@mpoke @sainoe