Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

Discuss security implications of account address #5

Open
okwme opened this issue Jan 28, 2021 · 0 comments
Open

Discuss security implications of account address #5

okwme opened this issue Jan 28, 2021 · 0 comments

Comments

@okwme
Copy link
Contributor

okwme commented Jan 28, 2021

taken from https://github.com/chainapsis/cosmos-sdk-interchain-account/blob/master/x/ibc-account/keeper/account.go#L17-L25:

	account := k.accountKeeper.GetAccount(ctx, address)
	// TODO: Discuss the vulnerabilities when creating a new account only if the old account does not exist
	// Attackers can interrupt creating accounts by sending some assets before the packet is delivered.
	// So it is needed to check that the account is not created from users.
	// Returns an error only if the account was created by other chain.
	// We need to discuss how we can judge this case.
	if account != nil {
		return nil, sdkerrors.Wrap(types.ErrAccountAlreadyExist, account.String())
	}

I don't think it should be a problem to allow the account address to already exist. In fact you may want to send money to it beforehand so you know it can start executing transactions from the get go. A bit like you need to send Ether to a contract based wallet on Ethereum.

The risk that I don't fully understand is how likely is it to brute force account generation with the Interchain Account module in order to get a collision on a PK controlled account with funds. This seems close to impossible if it is being prefixed by a hash of the port and channel. On top of that a salt is being used for the account (althought I might want to make a suggestion to remove the salt and allow the sending chain to be more specific about doing it's own namespacing. I haven't looked into the accountAddressQueue stuff yet so I'll follow up once I've taken a deeper look)

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

No branches or pull requests

1 participant