From b2c147216456acf56b4987c405446a55cb479752 Mon Sep 17 00:00:00 2001 From: krigga Date: Thu, 21 Sep 2023 11:04:37 +0300 Subject: [PATCH] fix: check bip32 path --- doc/COMMANDS.md | 9 +++++++-- src/common/bip32_check.c | 11 +++++++++++ src/common/bip32_check.h | 11 +++++++++++ src/handler/get_public_key.c | 5 +++++ src/handler/sign_data.c | 10 ++++++++-- src/handler/sign_tx.c | 10 ++++++++-- src/proof/proof_deserialize.c | 6 ++++++ src/sw.h | 4 ++++ tests/application_client/ton_command_sender.py | 1 + tests/test_pubkey_cmd.py | 14 +++++++++++++- 10 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 src/common/bip32_check.c create mode 100644 src/common/bip32_check.h diff --git a/doc/COMMANDS.md b/doc/COMMANDS.md index 4ed502a..fb15732 100644 --- a/doc/COMMANDS.md +++ b/doc/COMMANDS.md @@ -47,6 +47,8 @@ Use P2 to control what kind of address to present to user: * set bit 0x01 to make address testnet only * set bit 0x02 to use masterchain instead of basechain for the address +The bip32 path must be at least 3 elements long and must start with the prefix `m/44'/607'/`. + | CLA | INS | P1 | P2 | Lc | CData | | --- | --- | --- | --- | --- | --- | | 0xE0 | 0x05 | 0x00 (no display)
0x01 (display) | 0x00-0x03 | 1 + 4n | `len(bip32_path) (1)` \|\|
`bip32_path{1} (4)` \|\|
`...` \|\|
`bip32_path{n} (4)` | @@ -61,7 +63,7 @@ Use P2 to control what kind of address to present to user: ### Command -Sent as series of packages. First one contains bip32 path: +Sent as series of packages. First one contains bip32 path, which must be at least 3 elements long and must start with the prefix `m/44'/607'/`: | CLA | INS | P1 | P2 | Lc | CData | | --- | --- | --- | --- | --- | --- | @@ -87,6 +89,8 @@ Use P2 to control what kind of address to present to user: * set bit 0x01 to make address testnet only * set bit 0x02 to use masterchain instead of basechain for the address +The bip32 path must be at least 3 elements long and must start with the prefix `m/44'/607'/`. + Payload's length is implicitly calculated from buffer length. Proofs are generated according to this [spec](https://github.com/ton-blockchain/ton-connect/blob/main/requests-responses.md#address-proof-signature-ton_proof). @@ -107,7 +111,7 @@ Proofs are generated according to this [spec](https://github.com/ton-blockchain/ Signatures are generated according to this [spec](https://github.com/ton-blockchain/ton-connect/blob/main/requests-responses.md#sign-data-experimental). -Sent as series of packages. First one contains bip32 path: +Sent as series of packages. First one contains bip32 path, which must be at least 3 elements long and must start with the prefix `m/44'/607'/`: | CLA | INS | P1 | P2 | Lc | CData | | --- | --- | --- | --- | --- | --- | @@ -144,4 +148,5 @@ Then an arbitrary number of chunks with serialized custom data (see [CUSTOM_DATA | 0xB007 | `SW_BAD_STATE` | Security issue with bad state | | 0xB008 | `SW_SIGNATURE_FAIL` | Signature of raw transaction failed | | 0xB00B | `SW_REQUEST_TOO_LONG` | The request is too long | +| 0xB0BD | `SW_BAD_BIP32_PATH` | The bip32 derivation path is invalid | | 0x9000 | `OK` | Success | diff --git a/src/common/bip32_check.c b/src/common/bip32_check.c new file mode 100644 index 0000000..2dcdcb2 --- /dev/null +++ b/src/common/bip32_check.c @@ -0,0 +1,11 @@ +#include + +#include "../globals.h" + +#include "bip32_check.h" + +bool check_global_bip32_path() { + if (G_context.bip32_path_len <= 2) return false; + + return G_context.bip32_path[0] == 0x8000002c && G_context.bip32_path[1] == 0x8000025f; +} diff --git a/src/common/bip32_check.h b/src/common/bip32_check.h new file mode 100644 index 0000000..4647628 --- /dev/null +++ b/src/common/bip32_check.h @@ -0,0 +1,11 @@ +#pragma once + +#include // bool + +/** + * Check the bip32 path stored in G_context. + * + * @return true if bip32 path is valid, false otherwise + * + */ +bool check_global_bip32_path(); diff --git a/src/handler/get_public_key.c b/src/handler/get_public_key.c index e15da64..43468b4 100644 --- a/src/handler/get_public_key.c +++ b/src/handler/get_public_key.c @@ -29,6 +29,7 @@ #include "../sw.h" #include "../crypto.h" #include "../common/buffer.h" +#include "../common/bip32_check.h" #include "../ui/display.h" #include "../helper/send_response.h" @@ -42,6 +43,10 @@ int handler_get_public_key(uint8_t flags, buffer_t *cdata, bool display) { return io_send_sw(SW_WRONG_DATA_LENGTH); } + if (!check_global_bip32_path()) { + return io_send_sw(SW_BAD_BIP32_PATH); + } + if (crypto_derive_public_key(G_context.bip32_path, G_context.bip32_path_len, G_context.pk_info.raw_public_key) < 0) { diff --git a/src/handler/sign_data.c b/src/handler/sign_data.c index b852463..6bf8a3a 100644 --- a/src/handler/sign_data.c +++ b/src/handler/sign_data.c @@ -29,13 +29,12 @@ #include "../crypto.h" #include "../ui/display.h" #include "../common/buffer.h" +#include "../common/bip32_check.h" #include "../sign_data/sign_data_deserialize.h" int handler_sign_data(buffer_t *cdata, bool first, bool more) { if (first) { // first APDU, parse BIP32 path explicit_bzero(&G_context, sizeof(G_context)); - G_context.req_type = CONFIRM_SIGN_DATA; - G_context.state = STATE_NONE; if (!buffer_read_u8(cdata, &G_context.bip32_path_len) || !buffer_read_bip32_path(cdata, @@ -44,6 +43,13 @@ int handler_sign_data(buffer_t *cdata, bool first, bool more) { return io_send_sw(SW_WRONG_DATA_LENGTH); } + if (!check_global_bip32_path()) { + return io_send_sw(SW_BAD_BIP32_PATH); + } + + G_context.req_type = CONFIRM_SIGN_DATA; + G_context.state = STATE_NONE; + return io_send_sw(SW_OK); } diff --git a/src/handler/sign_tx.c b/src/handler/sign_tx.c index 00c4021..287bbd9 100644 --- a/src/handler/sign_tx.c +++ b/src/handler/sign_tx.c @@ -29,6 +29,7 @@ #include "../crypto.h" #include "../ui/display.h" #include "../common/buffer.h" +#include "../common/bip32_check.h" #include "../transaction/types.h" #include "../transaction/deserialize.h" #include "../transaction/hash.h" @@ -36,8 +37,6 @@ int handler_sign_tx(buffer_t *cdata, bool first, bool more) { if (first) { // first APDU, parse BIP32 path explicit_bzero(&G_context, sizeof(G_context)); - G_context.req_type = CONFIRM_TRANSACTION; - G_context.state = STATE_NONE; if (!buffer_read_u8(cdata, &G_context.bip32_path_len) || !buffer_read_bip32_path(cdata, @@ -46,6 +45,13 @@ int handler_sign_tx(buffer_t *cdata, bool first, bool more) { return io_send_sw(SW_WRONG_DATA_LENGTH); } + if (!check_global_bip32_path()) { + return io_send_sw(SW_BAD_BIP32_PATH); + } + + G_context.req_type = CONFIRM_TRANSACTION; + G_context.state = STATE_NONE; + return io_send_sw(SW_OK); } diff --git a/src/proof/proof_deserialize.c b/src/proof/proof_deserialize.c index e750cb8..4c1e994 100644 --- a/src/proof/proof_deserialize.c +++ b/src/proof/proof_deserialize.c @@ -7,6 +7,7 @@ #include "proof_deserialize.h" #include "../common/buffer.h" +#include "../common/bip32_check.h" #include "../types.h" #include "../globals.h" #include "../io.h" @@ -31,6 +32,11 @@ bool deserialize_proof(buffer_t *cdata, uint8_t flags) { return false; } + if (!check_global_bip32_path()) { + io_send_sw(SW_BAD_BIP32_PATH); + return false; + } + if (crypto_derive_public_key(G_context.bip32_path, G_context.bip32_path_len, G_context.proof_info.raw_public_key) < 0) { diff --git a/src/sw.h b/src/sw.h index 4481bad..9d67ae1 100644 --- a/src/sw.h +++ b/src/sw.h @@ -64,3 +64,7 @@ * Status word for a request that is too long. */ #define SW_REQUEST_TOO_LONG 0xB00B +/** + * Status word for bad bip32 path. + */ +#define SW_BAD_BIP32_PATH 0xB0BD diff --git a/tests/application_client/ton_command_sender.py b/tests/application_client/ton_command_sender.py index f3ddd30..cfede04 100644 --- a/tests/application_client/ton_command_sender.py +++ b/tests/application_client/ton_command_sender.py @@ -50,6 +50,7 @@ class Errors(IntEnum): SW_BAD_STATE = 0xB007 SW_SIGNATURE_FAIL = 0xB008 SW_REQUEST_TOO_LONG = 0xB00B + SW_BAD_BIP32_PATH = 0XB0BD class AddressDisplayFlags(IntFlag): NONE = 0 diff --git a/tests/test_pubkey_cmd.py b/tests/test_pubkey_cmd.py index f327c1b..1ad80c2 100644 --- a/tests/test_pubkey_cmd.py +++ b/tests/test_pubkey_cmd.py @@ -76,4 +76,16 @@ def test_get_public_key_confirm_refused(firmware, backend, navigator, test_name) instructions) # Assert that we have received a refusal assert e.value.status == Errors.SW_DENY - assert len(e.value.data) == 0 \ No newline at end of file + assert len(e.value.data) == 0 + +def test_get_public_key_bad_path(firmware, backend, navigator, test_name): + client = BoilerplateCommandSender(backend) + paths = ["m/44'/608'/0'/0'/0'/0'", "m/44'/607'"] + + for path in paths: + with pytest.raises(ExceptionRAPDU) as e: + with client.get_public_key_with_confirmation(path, AddressDisplayFlags.NONE): + pass + # Assert that we have received a refusal + assert e.value.status == Errors.SW_BAD_BIP32_PATH + assert len(e.value.data) == 0