From 54ad83ff50d8a14e4f95ac5279c3f9fc15aa9c35 Mon Sep 17 00:00:00 2001 From: Dimitrios Siganos Date: Thu, 14 Apr 2022 19:56:57 +0100 Subject: [PATCH] Improve public_key::decode_account to not write result on error (#3794) public_key::decode_account() would overwrite the object it was called on error, if the error was with the checksum part of the address. This commit changes the function to work on a temporary object and store the result only on success. resolves #3793 --- nano/core_test/uint256_union.cpp | 14 ++++++++++++++ nano/lib/numbers.cpp | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/nano/core_test/uint256_union.cpp b/nano/core_test/uint256_union.cpp index 89300e16bb..975ba86586 100644 --- a/nano/core_test/uint256_union.cpp +++ b/nano/core_test/uint256_union.cpp @@ -375,6 +375,20 @@ TEST (uint256_union, decode_nano_variant) ASSERT_FALSE (key.decode_account ("nano_1111111111111111111111111111111111111111111111111111hifc8npp")); } +/** + * It used to be the case that when the address was wrong only in the checksum part + * then the decode_account would return error and it would also write the address with + * fixed checksum into 'key', which is not desirable. + */ +TEST (uint256_union, key_is_not_updated_on_checksum_error) +{ + nano::account key; + ASSERT_EQ (key, 0); + bool result = key.decode_account ("nano_3e3j5tkog48pnny9dmfzj1r16pg8t1e76dz5tmac6iq689wyjfpiij4txtd1"); + ASSERT_EQ (key, 0); + ASSERT_TRUE (result); +} + TEST (uint256_union, account_transcode) { nano::account value; diff --git a/nano/lib/numbers.cpp b/nano/lib/numbers.cpp index aedd16a027..47d92a8c76 100644 --- a/nano/lib/numbers.cpp +++ b/nano/lib/numbers.cpp @@ -116,14 +116,18 @@ bool nano::public_key::decode_account (std::string const & source_a) } if (!error) { - *this = (number_l >> 40).convert_to (); + nano::public_key temp = (number_l >> 40).convert_to (); uint64_t check (number_l & static_cast (0xffffffffff)); uint64_t validation (0); blake2b_state hash; blake2b_init (&hash, 5); - blake2b_update (&hash, bytes.data (), bytes.size ()); + blake2b_update (&hash, temp.bytes.data (), temp.bytes.size ()); blake2b_final (&hash, reinterpret_cast (&validation), 5); error = check != validation; + if (!error) + { + *this = temp; + } } } else