diff --git a/src/libspark/coin.cpp b/src/libspark/coin.cpp index 27ca2f56e0..b7d87fd89b 100644 --- a/src/libspark/coin.cpp +++ b/src/libspark/coin.cpp @@ -52,6 +52,9 @@ Coin::Coin( std::vector padded_memo(memo_bytes); padded_memo.resize(this->params->get_memo_bytes()); + // Prepend the unpadded memo length + padded_memo.insert(padded_memo.begin(), (unsigned char) memo.size()); + // // Type-specific elements // @@ -63,7 +66,7 @@ Coin::Coin( MintCoinRecipientData r; r.d = address.get_d(); r.k = k; - r.memo = std::string(padded_memo.begin(), padded_memo.end()); + r.padded_memo = std::string(padded_memo.begin(), padded_memo.end()); CDataStream r_stream(SER_NETWORK, PROTOCOL_VERSION); r_stream << r; this->r_ = AEAD::encrypt(address.get_Q1()*SparkUtils::hash_k(k), "Mint coin data", r_stream); @@ -73,7 +76,7 @@ Coin::Coin( r.v = v; r.d = address.get_d(); r.k = k; - r.memo = std::string(padded_memo.begin(), padded_memo.end()); + r.padded_memo = std::string(padded_memo.begin(), padded_memo.end()); CDataStream r_stream(SER_NETWORK, PROTOCOL_VERSION); r_stream << r; this->r_ = AEAD::encrypt(address.get_Q1()*SparkUtils::hash_k(k), "Spend coin data", r_stream); @@ -131,10 +134,16 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) { throw std::runtime_error("Unable to identify coin"); } + // Check that the memo length is valid + unsigned char memo_length = r.padded_memo[0]; + if (memo_length > this->params->get_memo_bytes()) { + throw std::runtime_error("Unable to identify coin"); + } + data.d = r.d; data.v = this->v; data.k = r.k; - data.memo = r.memo; + data.memo = std::string(r.padded_memo.begin() + 1, r.padded_memo.begin() + 1 + memo_length); // remove the encoded length and padding } else { SpendCoinRecipientData r; @@ -146,10 +155,16 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) { throw std::runtime_error("Unable to identify coin"); } + // Check that the memo length is valid + unsigned char memo_length = r.padded_memo[0]; + if (memo_length > this->params->get_memo_bytes()) { + throw std::runtime_error("Unable to identify coin"); + } + data.d = r.d; data.v = r.v; data.k = r.k; - data.memo = r.memo; + data.memo = std::string(r.padded_memo.begin() + 1, r.padded_memo.begin() + 1 + memo_length); // remove the encoded length and padding } // Validate the coin diff --git a/src/libspark/coin.h b/src/libspark/coin.h index e8e85c1ecd..df7b0c3112 100644 --- a/src/libspark/coin.h +++ b/src/libspark/coin.h @@ -33,7 +33,7 @@ struct RecoveredCoinData { struct MintCoinRecipientData { std::vector d; // encrypted diversifier Scalar k; // nonce - std::string memo; // memo + std::string padded_memo; // padded memo with prepended one-byte length ADD_SERIALIZE_METHODS; @@ -41,7 +41,7 @@ struct MintCoinRecipientData { inline void SerializationOp(Stream& s, Operation ser_action) { READWRITE(d); READWRITE(k); - READWRITE(memo); + READWRITE(padded_memo); } }; @@ -50,7 +50,7 @@ struct SpendCoinRecipientData { uint64_t v; // value std::vector d; // encrypted diversifier Scalar k; // nonce - std::string memo; // memo + std::string padded_memo; // padded memo with prepended one-byte length ADD_SERIALIZE_METHODS; @@ -59,7 +59,7 @@ struct SpendCoinRecipientData { READWRITE(v); READWRITE(d); READWRITE(k); - READWRITE(memo); + READWRITE(padded_memo); } }; @@ -119,14 +119,14 @@ class Coin { // Encrypted coin data is always of a fixed size that depends on coin type // Its tag and key commitment sizes are enforced during its deserialization - // For mint coins: encrypted diversifier (with size), encoded nonce, padded memo (with size) - // For spend coins: encoded value, encrypted diversifier (with size), encoded nonce, padded memo (with size) + // For mint coins: encrypted diversifier (with size), encoded nonce, padded memo (with size), unpadded memo length + // For spend coins: encoded value, encrypted diversifier (with size), encoded nonce, padded memo (with size), unpadded memo length READWRITE(r_); - if (type == COIN_TYPE_MINT && r_.ciphertext.size() != (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes())) { + if (type == COIN_TYPE_MINT && r_.ciphertext.size() != (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes() + 1)) { std::cout << "Data size " << r_.ciphertext.size() << " but expected " << (AES_BLOCKSIZE + SCALAR_ENCODING + params->get_memo_bytes()) << std::endl; throw std::invalid_argument("Cannot deserialize mint coin due to bad encrypted data"); } - if (type == COIN_TYPE_SPEND && r_.ciphertext.size() != 8 + (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes())) { + if (type == COIN_TYPE_SPEND && r_.ciphertext.size() != 8 + (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes() + 1)) { std::cout << "Data size " << r_.ciphertext.size() << " but expected " << (8 + AES_BLOCKSIZE + SCALAR_ENCODING + params->get_memo_bytes()) << std::endl; throw std::invalid_argument("Cannot deserialize spend coin due to bad encrypted data"); } diff --git a/src/libspark/params.cpp b/src/libspark/params.cpp index 466de7be00..b36280fb28 100644 --- a/src/libspark/params.cpp +++ b/src/libspark/params.cpp @@ -17,7 +17,7 @@ Params const* Params::get_default() { return instance.get(); } - std::size_t memo_bytes = 32; + std::size_t memo_bytes = 31; // 32 bytes after length prepending! std::size_t max_M_range = 16; std::size_t n_grootle = 8; std::size_t m_grootle = 5; @@ -37,7 +37,7 @@ Params const* Params::get_test() { return instance.get(); } - std::size_t memo_bytes = 32; + std::size_t memo_bytes = 31; // 32 bytes after length prepending! std::size_t max_M_range = 16; std::size_t n_grootle = 2; std::size_t m_grootle = 4; diff --git a/src/libspark/params.h b/src/libspark/params.h index e37855dab4..3b18f18ad5 100644 --- a/src/libspark/params.h +++ b/src/libspark/params.h @@ -50,7 +50,7 @@ class Params { GroupElement U; // Coin parameters - std::size_t memo_bytes; + std::size_t memo_bytes; // This MUST NOT exceed 256, since the length is encoded to 8 bits // Range proof parameters std::size_t max_M_range; diff --git a/src/libspark/test/coin_test.cpp b/src/libspark/test/coin_test.cpp index 522ea29f2a..f0bc8ac244 100644 --- a/src/libspark/test/coin_test.cpp +++ b/src/libspark/test/coin_test.cpp @@ -28,7 +28,8 @@ BOOST_AUTO_TEST_CASE(mint_identify_recover) const uint64_t i = 12345; const uint64_t v = 86; - const std::string memo = "Spam and eggs"; + const std::string memo = "Spam and eggs are a tasty dish!"; // maximum length + BOOST_CHECK_EQUAL(memo.size(), params->get_memo_bytes()); // Generate keys SpendKey spend_key(params); @@ -57,8 +58,8 @@ BOOST_AUTO_TEST_CASE(mint_identify_recover) BOOST_CHECK_EQUAL_COLLECTIONS(i_data.d.begin(), i_data.d.end(), address.get_d().begin(), address.get_d().end()); BOOST_CHECK_EQUAL(i_data.v, v); BOOST_CHECK_EQUAL(i_data.k, k); - BOOST_CHECK_EQUAL(strcmp(memo.c_str(), i_data.memo.c_str()), 0); // compare strings in a lexicographical manner, as we pad the memo in the coin - BOOST_CHECK_EQUAL(i_data.memo.size(), params->get_memo_bytes()); // check that it is padded + BOOST_CHECK_EQUAL(i_data.memo, memo); + // Recover coin RecoveredCoinData r_data = coin.recover(full_view_key, i_data); BOOST_CHECK_EQUAL( @@ -105,8 +106,7 @@ BOOST_AUTO_TEST_CASE(spend_identify_recover) BOOST_CHECK_EQUAL_COLLECTIONS(i_data.d.begin(), i_data.d.end(), address.get_d().begin(), address.get_d().end()); BOOST_CHECK_EQUAL(i_data.v, v); BOOST_CHECK_EQUAL(i_data.k, k); - BOOST_CHECK_EQUAL(strcmp(memo.c_str(), i_data.memo.c_str()), 0); // compare strings in a lexicographical manner, as we pad the memo in the coin - BOOST_CHECK_EQUAL(i_data.memo.size(), params->get_memo_bytes()); // check that it is padded + BOOST_CHECK_EQUAL(i_data.memo, memo); // Recover coin RecoveredCoinData r_data = coin.recover(full_view_key, i_data);