diff --git a/fuzzing/fuzz_tx_parser.cc b/fuzzing/fuzz_tx_parser.cc index c51bb0c..1dfb53e 100644 --- a/fuzzing/fuzz_tx_parser.cc +++ b/fuzzing/fuzz_tx_parser.cc @@ -14,13 +14,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { buffer_t buf = {.ptr = data, .size = size, .offset = 0}; transaction_t tx; parser_status_e status; + uint32_t bip32_path[5]; char version[3] = {0}; char tx_input_len[3] = {0}; char tx_output_len[3] = {0}; memset(&tx, 0, sizeof(tx)); - status = transaction_deserialize(&buf, &tx); + status = transaction_deserialize(&buf, &tx, bip32_path); if (status == PARSING_OK) { format_u64(version, sizeof(version), tx.version); diff --git a/src/handler/sign_tx.c b/src/handler/sign_tx.c index 45f57c5..31d2457 100644 --- a/src/handler/sign_tx.c +++ b/src/handler/sign_tx.c @@ -38,7 +38,8 @@ int handler_sign_tx(buffer_t *cdata, uint8_t type, bool more) { G_context.req_type = CONFIRM_TRANSACTION; G_context.state = STATE_NONE; - parser_status_e status = transaction_deserialize(cdata, &G_context.tx_info.transaction); + parser_status_e status = + transaction_deserialize(cdata, &G_context.tx_info.transaction, G_context.bip32_path); PRINTF("Header Parsing status: %d.\n", status); diff --git a/src/transaction/deserialize.c b/src/transaction/deserialize.c index 74b47d0..995450a 100644 --- a/src/transaction/deserialize.c +++ b/src/transaction/deserialize.c @@ -74,13 +74,17 @@ parser_status_e transaction_input_deserialize(buffer_t *buf, transaction_input_t return buf->size - buf->offset == 0 ? PARSING_OK : INPUT_PARSING_ERROR; } -parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) { +parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx, uint32_t *bip32_path) { + uint8_t n_output = 0; + uint8_t n_input = 0; + uint8_t change_address_type = 0; + uint32_t change_address_index = 0; + // Version, 2 bytes if (!buffer_read_u16(buf, &tx->version, BE)) { return VERSION_PARSING_ERROR; } - uint8_t n_output = 0; if (!buffer_read_u8(buf, &n_output)) { return OUTPUTS_LENGTH_PARSING_ERROR; } @@ -92,7 +96,6 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) { return OUTPUTS_LENGTH_PARSING_ERROR; } - uint8_t n_input = 0; if (!buffer_read_u8(buf, &n_input)) { return INPUTS_LENGTH_PARSING_ERROR; } @@ -104,5 +107,23 @@ parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx) { return INPUTS_LENGTH_PARSING_ERROR; } + if (!buffer_read_u8(buf, &change_address_type)) { + return HEADER_PARSING_ERROR; + } + + if (change_address_type != 0 && change_address_type != 1) { + return HEADER_PARSING_ERROR; + } + + if (!buffer_read_u32(buf, &change_address_index, BE)) { + return HEADER_PARSING_ERROR; + } + + bip32_path[0] = 0x8000002C; + bip32_path[1] = 0x8001b207; + bip32_path[2] = 0x80000000; + bip32_path[3] = (uint32_t)(change_address_type); + bip32_path[4] = change_address_index; + return buf->size - buf->offset == 0 ? PARSING_OK : HEADER_PARSING_ERROR; } diff --git a/src/transaction/deserialize.h b/src/transaction/deserialize.h index 2dde780..d57a079 100644 --- a/src/transaction/deserialize.h +++ b/src/transaction/deserialize.h @@ -10,11 +10,13 @@ * Pointer to buffer with serialized transaction. * @param[out] tx * Pointer to transaction structure. + * @param[out] bip32_path + * Pointer to bip32_path array where change address info will be set * * @return PARSING_OK if success, error status otherwise. * */ -parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx); +parser_status_e transaction_deserialize(buffer_t *buf, transaction_t *tx, uint32_t *bip32_path); parser_status_e transaction_output_deserialize(buffer_t *buf, transaction_output_t *txout); diff --git a/src/transaction/serialize.c b/src/transaction/serialize.c index 87a1723..6bdf494 100644 --- a/src/transaction/serialize.c +++ b/src/transaction/serialize.c @@ -7,7 +7,7 @@ #include "../common/write.h" #include "../common/varint.h" -int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len) { +int transaction_serialize(const transaction_t *tx, uint32_t *path, uint8_t *out, size_t out_len) { size_t offset = 0; if (out_len < 4) { @@ -19,6 +19,11 @@ int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len) out[offset++] = tx->tx_output_len; out[offset++] = tx->tx_input_len; + out[offset++] = (uint8_t) path[3]; + + write_u32_be(out, offset, path[4]); + offset += 4; + return (int) offset; } diff --git a/src/transaction/serialize.h b/src/transaction/serialize.h index 698ac0c..6c1ec24 100644 --- a/src/transaction/serialize.h +++ b/src/transaction/serialize.h @@ -11,6 +11,8 @@ * * @param[in] tx * Pointer to input transaction structure. + * @param[in] bip32_path + * Pointer to bip32_path array where change address data will be read * @param[out] out * Pointer to output byte buffer. * @param[in] out_len @@ -19,7 +21,7 @@ * @return number of bytes written if success, -1 otherwise. * */ -int transaction_serialize(const transaction_t *tx, uint8_t *out, size_t out_len); +int transaction_serialize(const transaction_t *tx, uint32_t *path, uint8_t *out, size_t out_len); /** * Serialize transaction output in byte buffer. diff --git a/src/transaction/tx_validate.c b/src/transaction/tx_validate.c index 72aeb18..88bb2a2 100644 --- a/src/transaction/tx_validate.c +++ b/src/transaction/tx_validate.c @@ -18,7 +18,6 @@ bool tx_validate_parsed_transaction(transaction_t* tx) { // Change address will always be output[1] if it exists if (tx->tx_output_len == 2) { transaction_output_t change_output = tx->tx_outputs[1]; - transaction_input_t first_utxo = tx->tx_inputs[0]; if (change_output.script_public_key[0] != 0x20) { // Change address can only be SCHNORR address and it's not @@ -31,11 +30,11 @@ bool tx_validate_parsed_transaction(transaction_t* tx) { // was validated when we deserialized the data. memmove(change_address_pubkey, change_output.script_public_key + 1, 32); + // Forcing these values. path[3] and path[4] + // would've been set by transaction_deserialize G_context.bip32_path[0] = 0x8000002C; G_context.bip32_path[1] = 0x8001b207; G_context.bip32_path[2] = 0x80000000; - G_context.bip32_path[3] = (uint32_t)(first_utxo.address_type); - G_context.bip32_path[4] = first_utxo.address_index; G_context.bip32_path_len = 5; diff --git a/tests/application_client/kaspa_transaction.py b/tests/application_client/kaspa_transaction.py index 733d924..07a5fa0 100644 --- a/tests/application_client/kaspa_transaction.py +++ b/tests/application_client/kaspa_transaction.py @@ -81,10 +81,14 @@ def __init__(self, version: int, inputs: list[TransactionInput], outputs: list[TransactionOutput], + change_address_type: int = 0, + change_address_index: int = 0, do_check: bool = True) -> None: self.version: int = version self.inputs: list[TransactionInput] = inputs self.outputs: list[TransactionOutput] = outputs + self.change_address_type: int = change_address_type + self.change_address_index: int = change_address_index if do_check: if not 0 <= self.version <= 1: @@ -94,7 +98,9 @@ def serialize_first_chunk(self) -> bytes: return b"".join([ self.version.to_bytes(2, byteorder="big"), len(self.outputs).to_bytes(1, byteorder="big"), - len(self.inputs).to_bytes(1, byteorder="big") + len(self.inputs).to_bytes(1, byteorder="big"), + self.change_address_type.to_bytes(1, byteorder="big"), + self.change_address_index.to_bytes(4, byteorder="big"), ]) def serialize(self) -> bytes: diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00000.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00000.png new file mode 100644 index 0000000..8842989 Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00000.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00001.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00001.png new file mode 100644 index 0000000..a0d6edf Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00001.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00002.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00002.png new file mode 100644 index 0000000..4fbea57 Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00002.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00003.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00003.png new file mode 100644 index 0000000..bc7573a Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00003.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00004.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00004.png new file mode 100644 index 0000000..655cdcc Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00004.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00005.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00005.png new file mode 100644 index 0000000..ddddd35 Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00005.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00006.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00006.png new file mode 100644 index 0000000..b8a4cf4 Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00006.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00007.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00007.png new file mode 100644 index 0000000..66c411c Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00007.png differ diff --git a/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00008.png b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00008.png new file mode 100644 index 0000000..277becc Binary files /dev/null and b/tests/snapshots/nanos/test_sign_tx_simple_change_idx1/00008.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00000.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00000.png new file mode 100644 index 0000000..df51419 Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00000.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00001.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00001.png new file mode 100644 index 0000000..81ff98e Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00001.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00002.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00002.png new file mode 100644 index 0000000..ef35add Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00002.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00003.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00003.png new file mode 100644 index 0000000..5271a83 Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00003.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00004.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00004.png new file mode 100644 index 0000000..48d98b4 Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00004.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00005.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00005.png new file mode 100644 index 0000000..53ae651 Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00005.png differ diff --git a/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00006.png b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00006.png new file mode 100644 index 0000000..5fefcf1 Binary files /dev/null and b/tests/snapshots/nanosp/test_sign_tx_simple_change_idx1/00006.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00000.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00000.png new file mode 100644 index 0000000..df51419 Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00000.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00001.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00001.png new file mode 100644 index 0000000..81ff98e Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00001.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00002.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00002.png new file mode 100644 index 0000000..ef35add Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00002.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00003.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00003.png new file mode 100644 index 0000000..5271a83 Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00003.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00004.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00004.png new file mode 100644 index 0000000..48d98b4 Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00004.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00005.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00005.png new file mode 100644 index 0000000..53ae651 Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00005.png differ diff --git a/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00006.png b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00006.png new file mode 100644 index 0000000..5fefcf1 Binary files /dev/null and b/tests/snapshots/nanox/test_sign_tx_simple_change_idx1/00006.png differ diff --git a/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00000.png b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00000.png new file mode 100644 index 0000000..259a4e9 Binary files /dev/null and b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00000.png differ diff --git a/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00001.png b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00001.png new file mode 100644 index 0000000..64791b3 Binary files /dev/null and b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00001.png differ diff --git a/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00002.png b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00002.png new file mode 100644 index 0000000..10d784e Binary files /dev/null and b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00002.png differ diff --git a/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00003.png b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00003.png new file mode 100644 index 0000000..7a05bf7 Binary files /dev/null and b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00003.png differ diff --git a/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00004.png b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00004.png new file mode 100644 index 0000000..7e6aefd Binary files /dev/null and b/tests/snapshots/stax/test_sign_tx_simple_change_idx1/00004.png differ diff --git a/tests/test_sign_cmd.py b/tests/test_sign_cmd.py index 0e3d9fd..2b7dae3 100644 --- a/tests/test_sign_cmd.py +++ b/tests/test_sign_cmd.py @@ -66,6 +66,75 @@ def test_sign_tx_simple(firmware, backend, navigator, test_name): assert transaction.get_sighash(0) == sighash assert check_signature_validity(public_key, der_sig, sighash, transaction) +def test_sign_tx_simple_change_idx1(firmware, backend, navigator, test_name): + # Use the app interface instead of raw interface + client = KaspaCommandSender(backend) + # The path used for this entire test + path: str = "m/44'/111111'/0'/0/0" + + # First we need to get the public key of the device in order to build the transaction + rapdu = client.get_public_key(path=path) + _, public_key, _, _ = unpack_get_public_key_response(rapdu.data) + + change_path: str = "m/44'/111111'/0'/1/1" + + # First we need to get the public key of the device in order to build the transaction + change_rapdu = client.get_public_key(path=change_path) + _, change_public_key, _, _ = unpack_get_public_key_response(change_rapdu.data) + + valid_change_script_public_key = b"".join([ + 0x20.to_bytes(1, 'big'), + change_public_key[1:33], + 0xac.to_bytes(1, 'big')]).hex() + + # Create the transaction that will be sent to the device for signing + transaction = Transaction( + version=0, + change_address_index=1, + change_address_type=1, + inputs=[ + TransactionInput( + value=1100000, + tx_id="40b022362f1a303518e2b49f86f87a317c87b514ca0f3d08ad2e7cf49d08cc70", + address_type=0, + address_index=0, + index=0, + public_key=public_key[1:33] + ) + ], + outputs=[ + TransactionOutput( + value=1090000, + script_public_key=valid_change_script_public_key + ) + ] + ) + + # Send the sign device instruction. + # As it requires on-screen validation, the function is asynchronous. + # It will yield the result when the navigation is done + with client.sign_tx(transaction=transaction): + # Validate the on-screen request by performing the navigation appropriate for this device + if firmware.device.startswith("nano"): + navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK, + [NavInsID.BOTH_CLICK], + "Approve", + ROOT_SCREENSHOT_PATH, + test_name) + else: + navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP, + [NavInsID.USE_CASE_REVIEW_CONFIRM, + NavInsID.USE_CASE_STATUS_DISMISS], + "Hold to sign", + ROOT_SCREENSHOT_PATH, + test_name) + + # The device as yielded the result, parse it and ensure that the signature is correct + response = client.get_async_response().data + _, _, _, der_sig, _, sighash = unpack_sign_tx_response(response) + assert transaction.get_sighash(0) == sighash + assert check_signature_validity(public_key, der_sig, sighash, transaction) + def test_sign_tx_with_change(firmware, backend, navigator, test_name): # Use the app interface instead of raw interface client = KaspaCommandSender(backend) diff --git a/unit-tests/test_tx_parser.c b/unit-tests/test_tx_parser.c index db34921..6a5b061 100644 --- a/unit-tests/test_tx_parser.c +++ b/unit-tests/test_tx_parser.c @@ -17,22 +17,26 @@ static void test_tx_serialization(void **state) { transaction_t tx; tx.tx_output_len = 2; tx.tx_input_len = 3; + + uint32_t path[5] = {0}; // clang-format off uint8_t raw_tx[] = { // header - 0x00, 0x01, 0x02, 0x03 + 0x00, 0x01, 0x02, 0x03, + 0x01, + 0x04, 0x05, 0x06, 0xFF }; buffer_t buf = {.ptr = raw_tx, .size = sizeof(raw_tx), .offset = 0}; - parser_status_e status = transaction_deserialize(&buf, &tx); + parser_status_e status = transaction_deserialize(&buf, &tx, path); assert_int_equal(status, PARSING_OK); assert_int_equal(tx.version, 1); uint8_t output[350]; - int length = transaction_serialize(&tx, output, sizeof(output)); + int length = transaction_serialize(&tx, path, output, sizeof(output)); assert_int_equal(length, sizeof(raw_tx)); assert_memory_equal(raw_tx, output, sizeof(raw_tx)); } @@ -42,7 +46,9 @@ static int run_test_tx_serialize(uint8_t* raw_tx, size_t raw_tx_len) { buffer_t buf = {.ptr = raw_tx, .size = raw_tx_len, .offset = 0}; - return transaction_deserialize(&buf, &tx); + uint32_t path[5] = {0}; + + return transaction_deserialize(&buf, &tx, path); } static void test_tx_deserialization_fail(void **state) { @@ -72,11 +78,16 @@ static void test_tx_deserialization_fail(void **state) { 0x00, 0x01, 0x02, 0x00 }; + uint8_t invalid_change_type[] = { + 0x00, 0x01, 0x02, 0x03, 0x02 + }; + assert_int_equal(run_test_tx_serialize(invalid_version, sizeof(invalid_version)), VERSION_PARSING_ERROR); assert_int_equal(run_test_tx_serialize(missing_outlen, sizeof(missing_outlen)), OUTPUTS_LENGTH_PARSING_ERROR); assert_int_equal(run_test_tx_serialize(invalid_outlen, sizeof(invalid_outlen)), OUTPUTS_LENGTH_PARSING_ERROR); assert_int_equal(run_test_tx_serialize(missing_inlen, sizeof(missing_inlen)), INPUTS_LENGTH_PARSING_ERROR); assert_int_equal(run_test_tx_serialize(invalid_inlen, sizeof(invalid_inlen)), INPUTS_LENGTH_PARSING_ERROR); + assert_int_equal(run_test_tx_serialize(invalid_change_type, sizeof(invalid_change_type)), HEADER_PARSING_ERROR); } static void test_tx_input_serialization(void **state) { @@ -309,8 +320,9 @@ static void test_serialization_fail(void **state) { transaction_input_t txin; uint8_t buffer[1] = {0}; + uint32_t path[5] = {0}; - assert_int_equal(transaction_serialize(&tx, buffer, sizeof(buffer)), -1); + assert_int_equal(transaction_serialize(&tx, path, buffer, sizeof(buffer)), -1); assert_int_equal(transaction_output_serialize(&txout, buffer, sizeof(buffer)), -1); assert_int_equal(transaction_input_serialize(&txin, buffer, sizeof(buffer)), -1); }