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

Risky IBC channel-id Validation #265

Open
4 tasks
Hellobloc opened this issue Jun 3, 2024 · 1 comment
Open
4 tasks

Risky IBC channel-id Validation #265

Hellobloc opened this issue Jun 3, 2024 · 1 comment

Comments

@Hellobloc
Copy link

Hellobloc commented Jun 3, 2024

Summary of Bug

This repo is an Evmos-based project, and Evmos uses a magic string to identify the opposing chain in IBC protocol to address this vulnerability.. But This practice is bad for their fork projects, as other projects can easily have differences in channel-id when building links with chains like ATOM, leading to incorrect implementations.

        //https://github.com/zama-ai/evmos/blob/main/x/claims/types/params.go#L40-L46
	DefaultAuthorizedChannels = []string{
		"channel-0", // Osmosis
		"channel-3", // Cosmos Hub
	}
	DefaultEVMChannels = []string{
		"channel-2", // Injective
	}

Failure to properly handle these channel-ids and claim methods may result in the above vulnerability not being fixed and still presenting the risk. This is because a malicious user can hijack the relevant channel-id to masquerade as an ATOM and Osmosis chain.

Impact

This issue may leave the claim method still at risk of the above vulnerability when this project's IBC and Claim function is alive.

Additional context

https://github.com/zama-ai/evmos/blob/main/x/claims/types/params.go#L146-L154
https://github.com/zama-ai/evmos/blob/main/x/claims/types/params.go#L25-L31
https://github.com/zama-ai/evmos/blob/main/x/claims/keeper/ibc_callbacks.go#L138-L140

Recommendations

We recommend removing the claim function when the claim method is not used or safeguard the consistency of chain-id to avoid potential risks resulting from future updates.


Please note that there are also issues related to the authentication of EVMchannels.

This means that IsEVMChannel() check are also at risk.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@Hellobloc Hellobloc changed the title Risk Report for Migration of Claimable Amount through IBC Risky IBC channel-id Validation Jun 3, 2024
Copy link

This issue is stale because it has been open 45 days with no activity. Remove Status: Stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant