Audit Address mapping #6723
Comments
|
A set of mnemonic words can generate either Ethereum's h160 address or Polkadot's SS58 address. As long as the bip44 path is determined, can't these two addresses be directly mapped? In this way, users only need to manage a set of mnemonic words. |
|
The problem is that we need a mapping that maps addresses. Not mnemonics or private keys. Not even a mapping between public keys is sufficient. A contract can pass an H160 as argument and you need to derive the account id from it. You don't even know the public key of that H160. |
|
Thank you for your answer. I am developing a polkavm wallet. The problem I am encountering now is whether I should create an ecdsa key pair or a sr25519 key pair for the user. Users coming from evm are more familiar with h160, but ecdsa seems to need to pass precompile to interact with the asset hub. maybe i need to create two accounts, ecdsa and sr25519, at the same time, but these two accounts are not related. Sorry, I'm not very familiar with how polkavm works yet. |
|
This is a tricky problem with a lot of nuance to it. Let me try to explain.
You might think that you need to send Eth transactions through our Eth RPC interface when you have an ethereum private key. However, that is not true. You can sign a normal AssetHub transaction with your Ethereum key. AssetHub (as most other Substrate based chain) accept multiple different signature types. One of them the ecdsa curve Ethereum uses. However, the problem with that is that the address this transaction will perceived to be coming from is based off the public key. The problem with that derivation is that it doesn't allow you to know somebodies AccountID32 from just looking at their H160. You would need the public key which is not available in all cases can't can't be recovered from the H160. This problem arises when running Eth contracts on a AccountId32 chain. This is why we introduced a new address to address mapping. Cross chain interactions with chains using H160 addresses (like Mythical or Moonbeam) face similar issues. To remedy the situation we should add another Please note that you could sign a transaction with a H160 key today. The downside is the AccountID32 your would be acting as would be different that as if you would signed a Eth tx with the same key. A suboptimal solution.
Our goal is that both keys can be used for both transaction type:
Then a wallet could allow using both flavors of transactions (and hence both flavors of Dapps) to be used from any key type. And your address would not change depending on which transaction type you use. All the wallets would need to add is allowing cross injecting of accounts: Allow injecting Eth keys into Polkadot native Dapps and injecting Polkadot Keys into Eth Dapps. The wallet would apply the canonical address mapping so the Dapps won't even be aware of whar is happening. |
|
Thank you again for your reply. It is true that I misunderstood the signature verification method of the substrate chain. Looks like I need to spend more time studying the documentation. This is not an audit question, sorry for taking up your time. |
Fixes #6723 ## Motivation Internal auditors recommended to not truncate Polkadot Addresses when deriving Ethereum addresses from it. Reasoning is that they are raw public keys where truncating could lead to collisions when weaknesses in those curves are discovered in the future. Additionally, some pallets generate account addresses in a way where only the suffix we were truncating contains any entropy. The changes in this PR act as a safe guard against those two points. ## Changes made We change the `to_address` function to first hash the AccountId32 and then use trailing 20 bytes as `AccountId20`. If the `AccountId32` ends with 12x 0xEE we keep our current behaviour of just truncating those trailing bytes. ## Security Discussion This will allow us to still recover the original `AccountId20` because those are constructed by just adding those 12 bytes. Please note that generating an ed25519 key pair where the trailing 12 bytes are 0xEE is theoretically possible as 96bits is not a huge search space. However, this cannot be used as an attack vector. It will merely allow this address to interact with `pallet_revive` without registering as the fallback account is the same as the actual address. The ultimate vanity address. In practice, this is not relevant since the 0xEE addresses are not valid public keys for sr25519 which is used almost everywhere. tl:dr: We keep truncating in case of an Ethereum address derived account id. This is safe as those are already derived via keccak. In every other case where we have to assume that the account id might be a public key. Therefore we first hash and then take the trailing bytes. ## Do we need a Migration for Westend No. We changed the name of the mapping. This means the runtime will not try to read the old data. Ethereum keys are unaffected by this change. We just advise people to re-register their AccountId32 in case they need to use it as it is a very small circle of users (just 3 addresses registered). This will not cause disturbance on Westend. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #6723 Internal auditors recommended to not truncate Polkadot Addresses when deriving Ethereum addresses from it. Reasoning is that they are raw public keys where truncating could lead to collisions when weaknesses in those curves are discovered in the future. Additionally, some pallets generate account addresses in a way where only the suffix we were truncating contains any entropy. The changes in this PR act as a safe guard against those two points. We change the `to_address` function to first hash the AccountId32 and then use trailing 20 bytes as `AccountId20`. If the `AccountId32` ends with 12x 0xEE we keep our current behaviour of just truncating those trailing bytes. This will allow us to still recover the original `AccountId20` because those are constructed by just adding those 12 bytes. Please note that generating an ed25519 key pair where the trailing 12 bytes are 0xEE is theoretically possible as 96bits is not a huge search space. However, this cannot be used as an attack vector. It will merely allow this address to interact with `pallet_revive` without registering as the fallback account is the same as the actual address. The ultimate vanity address. In practice, this is not relevant since the 0xEE addresses are not valid public keys for sr25519 which is used almost everywhere. tl:dr: We keep truncating in case of an Ethereum address derived account id. This is safe as those are already derived via keccak. In every other case where we have to assume that the account id might be a public key. Therefore we first hash and then take the trailing bytes. No. We changed the name of the mapping. This means the runtime will not try to read the old data. Ethereum keys are unaffected by this change. We just advise people to re-register their AccountId32 in case they need to use it as it is a very small circle of users (just 3 addresses registered). This will not cause disturbance on Westend. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The EVM uses 20 byte addresses. Polkadot uses 32 byte addresses. In order to make those two components work together we needed to design and implement a mapping between the two. Our design works slightly different from frontier or Acala.
We need to get this audited as soon as possible as it might incur bigger changes we are not able to make easily once the chain is live.
Tasks
The text was updated successfully, but these errors were encountered: