immutability-io / vault-ethereum

A plugin that turns Vault into an Ethereum wallet.
243 stars 65 forks source link

When "address_to" is not mentioned during the transaction signing, the funds are transferred to NULL address. #70

Closed guruchaithanya closed 4 years ago

guruchaithanya commented 4 years ago

Detailed Description

When "address_to" is not mentioned during the transaction signing, the funds are transferred to 0x0 address.

I have signed below transaction and made the debit request. Payload for signing:

{ "amount":"200000000000000000", "to": "0x36D1F896E55a6577C62FDD6b84fbF74582266700", "data": "666f6f626172", "encoding": "hex" }

Signing Request:

curl -s --cacert ~/etc/vault.d/root.crt --header "X-Vault-Token: $VAULT_TOKEN" \ --request POST \ --data @payload.json \ https://localhost:8200/v1/ethereum/dev/accounts/test2/sign-tx | jq .

Signed Transaction:

{ "request_id": "9470677c-00e3-2d8d-2c37-31f207680524", "lease_id": "", "renewable": false, "lease_duration": 0, "data": { "address_from": "0xff93ff615bb9c9a354b3e5e29b416aab58b502d2", "address_to": "0x0000000000000000000000000000000000000000", "amount": "200000000000000000", "amount_in_usd": "0", "gas_limit": "21000", "gas_price": "1000000000", "signed_transaction": "0xf87102843b9aca008252089400000000000000000000000000000000000000008802c68af0bb14000086666f6f6261722ba088931d4492778dbf51474a05e286121d74017465587b5aadc946aae760f84f7ea07add37951665a703a17992586df632f5bb03d6b7f730a3b6bb4640fc60bab999", "starting_balance": 18350958000000000000, "starting_balance_in_usd": "0", "total_spend": "1600000000000000000", "transaction_hash": "0x00d310b7be5e4b565b8dc8982070a0fc9a410751ebc78e83c929d1b69c426114" }, "wrap_info": null, "warnings": null, "auth": null }

Context

This if not fixed could be a serious lapse, as the funds will be lost. Mostly in the dev testing this will be caught, but better to provide additional checks in the plugin itself.

Possible Implementation

Additional checks while signing the transaction.

Your Environment

cypherhat commented 4 years ago

Thanks for this. I agree that it should be fixed... So I fixed it