cryptonote_basic: remove legacy address parser
What changed, and why it matters
This commit removes an old, unused way of representing Monero wallet addresses. The old format used a simple checksum that added up bytes, which is weaker than modern formats. The change deletes the legacy parser and the test that checked it still worked. There is no direct evidence this was exploitable, but removing weak legacy code reduces future risk.
Treat as routine cleanup with minor defensive-security value. Reviewers should confirm no production code still depends on the removed legacy format, and that base58 address parsing still rejects malformed inputs correctly.
Security signals we found
Removal of legacy address parser with weak additive byte checksum
Deletion of packed binary address blob structures
Reduction of attack surface in address parsing path
No explicit security advisory or CVE referenced in commit
Evidence from the diff
The patch deletes the legacy public_address_outer_blob and public_integrated_address_outer_blob structures, their simple additive checksum helpers, the CRYPTONOTE_PUBLIC_ADDRESS_TEXTBLOB_VER constant, and the fallback hex-parsing branch in get_account_address_from_str(). Address parsing now relies solely on the base58-encoded format with its own checksum. A unit test for the old hex address format is also removed.
Changed components
src/cryptonote_basic/cryptonote_basic_impl.cppsrc/cryptonote_basic/cryptonote_basic_impl.hsrc/cryptonote_config.htests/unit_tests/base58.cppInspect captured patch +41 / −123
diff --git a/src/cryptonote_basic/cryptonote_basic_impl.cpp b/src/cryptonote_basic/cryptonote_basic_impl.cpp
index bd79a7b..045bbf8 100644
--- a/src/cryptonote_basic/cryptonote_basic_impl.cpp
+++ b/src/cryptonote_basic/cryptonote_basic_impl.cpp
@@ -127,26 +127,6 @@ namespace cryptonote {
return true;
}
//------------------------------------------------------------------------------------
- uint8_t get_account_address_checksum(const public_address_outer_blob& bl)
- {
- const unsigned char* pbuf = reinterpret_cast<const unsigned char*>(&bl);
- uint8_t summ = 0;
- for(size_t i = 0; i!= sizeof(public_address_outer_blob)-1; i++)
- summ += pbuf[i];
-
- return summ;
- }
- //------------------------------------------------------------------------------------
- uint8_t get_account_integrated_address_checksum(const public_integrated_address_outer_blob& bl)
- {
- const unsigned char* pbuf = reinterpret_cast<const unsigned char*>(&bl);
- uint8_t summ = 0;
- for(size_t i = 0; i!= sizeof(public_integrated_address_outer_blob)-1; i++)
- summ += pbuf[i];
-
- return summ;
- }
- //-----------------------------------------------------------------------
std::string get_account_address_as_str(
network_type nettype
, bool subaddress
@@ -193,96 +173,60 @@ namespace cryptonote {
uint64_t integrated_address_prefix = get_config(nettype).CRYPTONOTE_PUBLIC_INTEGRATED_ADDRESS_BASE58_PREFIX;
uint64_t subaddress_prefix = get_config(nettype).CRYPTONOTE_PUBLIC_SUBADDRESS_BASE58_PREFIX;
- if (2 * sizeof(public_address_outer_blob) != str.size())
+ blobdata data;
+ uint64_t prefix;
+ if (!tools::base58::decode_addr(str, prefix, data))
{
- blobdata data;
- uint64_t prefix;
- if (!tools::base58::decode_addr(str, prefix, data))
- {
- LOG_PRINT_L2("Invalid address format");
- return false;
- }
-
- if (integrated_address_prefix == prefix)
- {
- info.is_subaddress = false;
- info.has_payment_id = true;
- }
- else if (address_prefix == prefix)
- {
- info.is_subaddress = false;
- info.has_payment_id = false;
- }
- else if (subaddress_prefix == prefix)
- {
- info.is_subaddress = true;
- info.has_payment_id = false;
- }
- else {
- LOG_PRINT_L1("Wrong address prefix: " << prefix << ", expected " << address_prefix
- << " or " << integrated_address_prefix
- << " or " << subaddress_prefix);
- return false;
- }
+ LOG_PRINT_L2("Invalid address format");
+ return false;
+ }
- if (info.has_payment_id)
- {
- integrated_address iadr;
- if (!::serialization::parse_binary(data, iadr))
- {
- LOG_PRINT_L1("Account public address keys can't be parsed");
- return false;
- }
- info.address = iadr.adr;
- info.payment_id = iadr.payment_id;
- }
- else
- {
- if (!::serialization::parse_binary(data, info.address))
- {
- LOG_PRINT_L1("Account public address keys can't be parsed");
- return false;
- }
- }
+ if (integrated_address_prefix == prefix)
+ {
+ info.is_subaddress = false;
+ info.has_payment_id = true;
+ }
+ else if (address_prefix == prefix)
+ {
+ info.is_subaddress = false;
+ info.has_payment_id = false;
+ }
+ else if (subaddress_prefix == prefix)
+ {
+ info.is_subaddress = true;
+ info.has_payment_id = false;
+ }
+ else {
+ LOG_PRINT_L1("Wrong address prefix: " << prefix << ", expected " << address_prefix
+ << " or " << integrated_address_prefix
+ << " or " << subaddress_prefix);
+ return false;
+ }
- if (!crypto::check_key(info.address.m_spend_public_key) || !crypto::check_key(info.address.m_view_public_key))
+ if (info.has_payment_id)
+ {
+ integrated_address iadr;
+ if (!::serialization::parse_binary(data, iadr))
{
- LOG_PRINT_L1("Failed to validate address keys");
+ LOG_PRINT_L1("Account public address keys can't be parsed");
return false;
}
+ info.address = iadr.adr;
+ info.payment_id = iadr.payment_id;
}
else
{
- // Old address format
- std::string buff;
- if(!string_tools::parse_hexstr_to_binbuff(str, buff))
- return false;
-
- if(buff.size()!=sizeof(public_address_outer_blob))
- {
- LOG_PRINT_L1("Wrong public address size: " << buff.size() << ", expected size: " << sizeof(public_address_outer_blob));
- return false;
- }
-
- public_address_outer_blob blob = *reinterpret_cast<const public_address_outer_blob*>(buff.data());
-
-
- if(blob.m_ver > CRYPTONOTE_PUBLIC_ADDRESS_TEXTBLOB_VER)
- {
- LOG_PRINT_L1("Unknown version of public address: " << blob.m_ver << ", expected " << CRYPTONOTE_PUBLIC_ADDRESS_TEXTBLOB_VER);
- return false;
- }
-
- if(blob.check_sum != get_account_address_checksum(blob))
+ if (!::serialization::parse_binary(data, info.address))
{
- LOG_PRINT_L1("Wrong public address checksum");
+ LOG_PRINT_L1("Account public address keys can't be parsed");
return false;
}
+ }
- //we success
- info.address = blob.m_address;
- info.is_subaddress = false;
- info.has_payment_id = false;
+ if (!crypto::check_key(info.address.m_spend_public_key) || !crypto::check_key(info.address.m_view_public_key))
+ {
+ LOG_PRINT_L1("Failed to validate address keys");
+ return false;
}
return true;
diff --git a/src/cryptonote_basic/cryptonote_basic_impl.h b/src/cryptonote_basic/cryptonote_basic_impl.h
index dc77858..ff7c59c 100644
--- a/src/cryptonote_basic/cryptonote_basic_impl.h
+++ b/src/cryptonote_basic/cryptonote_basic_impl.h
@@ -40,22 +40,6 @@ namespace cryptonote {
/* */
/************************************************************************/
-#pragma pack(push, 1)
- struct public_address_outer_blob
- {
- uint8_t m_ver;
- account_public_address m_address;
- uint8_t check_sum;
- };
- struct public_integrated_address_outer_blob
- {
- uint8_t m_ver;
- account_public_address m_address;
- crypto::hash8 payment_id;
- uint8_t check_sum;
- };
-#pragma pack (pop)
-
namespace
{
inline std::string return_first_address(const std::string &url, const std::vector<std::string> &addresses, bool dnssec_valid)
@@ -80,8 +64,6 @@ namespace cryptonote {
size_t get_min_block_weight(uint8_t version);
size_t get_max_tx_size();
bool get_block_reward(size_t median_weight, size_t current_block_weight, uint64_t already_generated_coins, uint64_t &reward, uint8_t version);
- uint8_t get_account_address_checksum(const public_address_outer_blob& bl);
- uint8_t get_account_integrated_address_checksum(const public_integrated_address_outer_blob& bl);
std::string get_account_address_as_str(
network_type nettype
@@ -150,4 +132,3 @@ namespace cryptonote {
}
bool parse_hash256(const std::string &str_hash, crypto::hash& hash);
-
diff --git a/src/cryptonote_config.h b/src/cryptonote_config.h
index e60d356..38f78da 100644
--- a/src/cryptonote_config.h
+++ b/src/cryptonote_config.h
@@ -40,7 +40,6 @@
#define CRYPTONOTE_MAX_BLOCK_NUMBER 500000000
#define CRYPTONOTE_MAX_TX_SIZE 1000000
#define CRYPTONOTE_MAX_TX_PER_BLOCK 0x10000000
-#define CRYPTONOTE_PUBLIC_ADDRESS_TEXTBLOB_VER 0
#define CRYPTONOTE_MINED_MONEY_UNLOCK_WINDOW 60
#define CURRENT_TRANSACTION_VERSION 2
#define CURRENT_BLOCK_MAJOR_VERSION 1
diff --git a/tests/unit_tests/base58.cpp b/tests/unit_tests/base58.cpp
index f105a04..37ded65 100644
--- a/tests/unit_tests/base58.cpp
+++ b/tests/unit_tests/base58.cpp
@@ -532,9 +532,3 @@ TEST(get_account_address_from_str, fails_on_invalid_address_view_key)
cryptonote::address_parse_info info;
ASSERT_FALSE(cryptonote::get_account_address_from_str(info, cryptonote::MAINNET, addr_str));
}
-
-TEST(get_account_address_from_str, parses_old_address_format)
-{
- cryptonote::address_parse_info info;
- ASSERT_TRUE(cryptonote::get_account_address_from_str(info, cryptonote::MAINNET, "002391bbbb24dea6fd95232e97594a27769d0153d053d2102b789c498f57a2b00b69cd6f2f5c529c1660f2f4a2b50178d6640c20ce71fe26373041af97c5b10236fc"));
-}
Why this scored 16/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.