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

ibc: Missing check in ClientState::new #4548

Closed
avahowell opened this issue Jun 4, 2024 · 1 comment
Closed

ibc: Missing check in ClientState::new #4548

avahowell opened this issue Jun 4, 2024 · 1 comment
Assignees
@avahowell
Labels
_P-V1 Priority: slated for V1 release zellic-ibc-needs-remediation Finding by Zellic reviewing the IBC component, needs remediation
Milestone
Sprint 8

Comments

@avahowell
Copy link
Contributor

Describe the bug
The ClientState::new function in ibc-types shares most of its checks with ibc-go's ClientState.Validate but omits the check that chain_id is nonempty, and none of ChainId's From/FromStr trait implementations contain this check. This will result in penumbra-ibc nodes accepting chain IDs that ibc-go nodes will reject (though when referring to the chain ID "", it will reserialize it as "-0", which ibc-go will accept).

The ibc-go library checks that the chain ID is nonempty.

Expected behavior

This likely does not have any security impact, but it is a deviation from the specification and can potentially lead to an issue in the future.
@avahowell avahowell added the zellic-ibc-needs-remediation Finding by Zellic reviewing the IBC component, needs remediation label Jun 4, 2024
@avahowell avahowell self-assigned this Jun 4, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Penumbra Jun 4, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label Jun 4, 2024
@aubrika aubrika added _P-V1 Priority: slated for V1 release and removed needs-refinement unclear, incomplete, or stub issue that needs work labels Jun 4, 2024
@aubrika aubrika moved this from Backlog to Todo in Penumbra Jun 5, 2024
@aubrika aubrika added this to the Sprint 8 milestone Jun 5, 2024
@erwanor
Copy link
Member

erwanor commented Jun 23, 2024

This specific addition was done in penumbra-zone/ibc-types#88

@erwanor erwanor closed this as completed Jun 23, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Penumbra Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@avahowell avahowell

Labels
_P-V1 Priority: slated for V1 release zellic-ibc-needs-remediation Finding by Zellic reviewing the IBC component, needs remediation
Projects
Penumbra
Archived in project
Milestone
Sprint 8
Development

No branches or pull requests
3 participants
@erwanor @avahowell @aubrika