search

ava-labs / hypersdk

Opinionated Framework for Building Hyper-Scalable Blockchains on Avalanche
https://hypersdk.xyz/
Other
193 stars 100 forks source link

[Discussion] Securely decode transaction binaries in the wallet UI #1197

Open containerman17 opened 1 month ago

containerman17 commented 1 month ago

This discussion is part of an ongoing prototype to use Metamask Snaps for a quick and easy wallet implementation in HyperSDK. Our goal is to bring HyperSDK to users efficiently. As an alternative, we considered EIP-712 signing, which is widely supported in EVM wallets, but it requires too many hacks and workarounds.

It is also applicable to future Core wallet integration.

Problem Statement

We aim to create a single Metamask Snap that supports [almost] all subnets on HyperSDK. This Snap won't know every subnet action, meaning it can't parse and label binary transactions for user signing.

Background:

Example Attack:

Current Solutions

Solidity Contracts:

EIP-712 Signing:

Proposed Solution: Action Hash

Benefits:

Required Modifications:

  1. Extend action ID from 1 to 4 bytes.
  2. Replace chain.Action.GetTypeID method with an automatic hash.
  3. Add an RPC method or JSON export with all action structural information (possibly unnecessary if actions are packed into WASM and brought to clients in a browser).

Alternatives:

  1. Add field names and lengths to the encoding of every field of every action (likely doubles transaction size).
  2. Hardcode one derivation path to one website (private keys from a website aren't used elsewhere, ensuring no point of misleading the user).
  3. Sign EIP-712 representation of the transaction (no transaction size bloat, but slows verification and tightens coupling of auth package with the chain.Transaction struct).
  4. Add a checksum of every action into a signature without including it in the transaction body (requires parsing the transaction and adding checksums before verification).

Perfect Outcome:

Until this is resolved, we will proceed with the one Metamask Snap per VM route.

containerman17 commented 1 month ago

4 bytes are not enough

Here is a list of methods that collide with the ERC20's 4-byte transfer(address,uint256) method signature. None of them currently replicate the 52-byte length (20+32 bytes), but it's only a matter of time.

ID Text Signature Bytes Signature
1111734 workMyDirefulOwner(uint256,uint256) 0xa9059cbb
844280 join_tg_invmru_haha_fd06787(address,bool) 0xa9059cbb
313067 func_2093253501(bytes) 0xa9059cbb
161159 transfer(bytes4[9],bytes5[6],int48[11]) 0xa9059cbb
31780 many_msg_babbage(bytes1) 0xa9059cbb
145 transfer(address,uint256) 0xa9059cbb

Source

~An 8-byte hash should be probably sufficient~

containerman17 commented 1 month ago
Screenshot 2024-07-26 at 11 35 22
containerman17 commented 3 weeks ago

Current Proposal

We create a JSON file containing all the actions of a VM, which we'll call an ABI. The VM will have just one ABI that includes all its actions. A wallet receives this JSON with data, for example:

{
"to": "morpheuslqztfzdOeeyp4evmowolede688pxry662n2kwewc0094p7r89ws56wmOu542",
"value": "1234567890",
"memo": "SGV5HRoZXJlIQ=="
}

And an ABI, for example:

[
  {
    "id": 0,
    "name": "Transfer",
    "types": {
      "Transfer": [
        {
          "name": "to",
          "type": "Address"
        },
        {
          "name": "value",
          "type": "uint64"
        },
        {
          "name": "memo",
          "type": "[]uint8"
        }
      ]
    }
  }
]

The ABI format is heavily influenced by EIP712, using Go language types.

With this information, a wallet (like Metamask Snap or Core) constructs two things:

  1. A UI to display to the user.
  2. A binary. The wallet knows the order and method to pack fields into binary format to sign the transaction.

Here's an example of a wallet screenshot. You can play around with the current implementation at ec2-18-224-139-0.us-east-2.compute.amazonaws.com

Screenshot 2024-08-16 at 13 53 46

ABI Validity Check

Signing with an ABI provided by a website is not much better than blind signing, so we need a way to validate the ABI.

Here are four options:

  1. Public static ABI registry: A community-maintained list of ChainIds and their respective ABI hashes on Github.
  2. Onchain ABI registry: Similar to the first option, but updates are made via a warp message from the majority validators of an L1.
  3. Check on auth level: Include a signed ABI hash in the transaction. This ensures that a transaction with an incorrect ABI will never go through. No changes to transaction structure are needed as this would be handled entirely within the auth package boundaries. However, this could cause transaction failures during upgrades.
  4. Add ABI hash for all transactions: The most radical option, adding 32 bytes to every transaction, even those signed by the backend.

I propose option 1 because it requires almost no effort.

containerman17 commented 3 weeks ago

Tasks and Pull Requests Related to the Frontend Implementation

WIP:

Blocked:

Done:

May be: