From 4512bb78000e61d0a001605a814570c80761f8e7 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 4 Dec 2023 17:24:57 -0600 Subject: [PATCH 1/3] Add size checks to coin deserialization --- src/libspark/aead.h | 12 ++++++++++++ src/libspark/coin.h | 17 +++++++++++++++++ src/libspark/util.cpp | 3 +++ 3 files changed, 32 insertions(+) diff --git a/src/libspark/aead.h b/src/libspark/aead.h index ce8470a17d..f5af124d49 100644 --- a/src/libspark/aead.h +++ b/src/libspark/aead.h @@ -15,8 +15,20 @@ struct AEADEncryptedData { template inline void SerializationOp(Stream& s, Operation ser_action) { READWRITE(ciphertext); + + // Tag must be the correct size READWRITE(tag); + if (tag.size() != AEAD_TAG_SIZE) { + std::cout << "Bad tag size " << tag.size() << std::endl; + throw std::invalid_argument("Cannot deserialize AEAD data due to bad tag"); + } + + // Key commitment must be the correct size, which also includes an encoded size READWRITE(key_commitment); + if (key_commitment.size() != 1 + AEAD_COMMIT_SIZE) { + std::cout << "Bad keycom size " << key_commitment.size() << std::endl; + throw std::invalid_argument("Cannot deserialize AEAD data due to bad key commitment size"); + } } }; diff --git a/src/libspark/coin.h b/src/libspark/coin.h index cdb42d336f..e8e85c1ecd 100644 --- a/src/libspark/coin.h +++ b/src/libspark/coin.h @@ -108,11 +108,28 @@ class Coin { ADD_SERIALIZE_METHODS; template inline void SerializationOp(Stream& s, Operation ser_action) { + // The type must be valid READWRITE(type); + if (type != COIN_TYPE_MINT && type != COIN_TYPE_SPEND) { + throw std::invalid_argument("Cannot deserialize coin due to bad type"); + } READWRITE(S); READWRITE(K); READWRITE(C); + + // 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) READWRITE(r_); + if (type == COIN_TYPE_MINT && r_.ciphertext.size() != (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes())) { + 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())) { + 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"); + } if (type == COIN_TYPE_MINT) { READWRITE(v); diff --git a/src/libspark/util.cpp b/src/libspark/util.cpp index 4547251320..17212cc1fd 100644 --- a/src/libspark/util.cpp +++ b/src/libspark/util.cpp @@ -36,6 +36,9 @@ uint64_t SparkUtils::diversifier_decrypt(const std::vector& key, if (key.size() != AES256_KEYSIZE) { throw std::invalid_argument("Bad diversifier decryption key size"); } + if (d.size() != AES_BLOCKSIZE) { + throw std::invalid_argument("Bad diversifier ciphertext size"); + } std::vector iv; iv.resize(AES_BLOCKSIZE); From 1eab7c7af8ded87b6b0ca0441b780b973c4cbe99 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 5 Dec 2023 11:30:51 -0600 Subject: [PATCH 2/3] Fix --- src/libspark/aead.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libspark/aead.h b/src/libspark/aead.h index f5af124d49..6f43a3d334 100644 --- a/src/libspark/aead.h +++ b/src/libspark/aead.h @@ -25,7 +25,7 @@ struct AEADEncryptedData { // Key commitment must be the correct size, which also includes an encoded size READWRITE(key_commitment); - if (key_commitment.size() != 1 + AEAD_COMMIT_SIZE) { + if (key_commitment.size() != AEAD_COMMIT_SIZE) { std::cout << "Bad keycom size " << key_commitment.size() << std::endl; throw std::invalid_argument("Cannot deserialize AEAD data due to bad key commitment size"); } From 28a85f5196ef4ff8009d83e4b68179cf222c311d Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Tue, 5 Dec 2023 11:41:08 -0600 Subject: [PATCH 3/3] Add AEAD deserialization test --- src/libspark/test/aead_test.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/libspark/test/aead_test.cpp b/src/libspark/test/aead_test.cpp index 2a3901326d..78e30bffe8 100644 --- a/src/libspark/test/aead_test.cpp +++ b/src/libspark/test/aead_test.cpp @@ -13,20 +13,28 @@ BOOST_AUTO_TEST_CASE(complete) GroupElement prekey; prekey.randomize(); - // Serialize + // Serialize message int message = 12345; - CDataStream ser(SER_NETWORK, PROTOCOL_VERSION); - ser << message; + CDataStream ser_message(SER_NETWORK, PROTOCOL_VERSION); + ser_message << message; // Encrypt - AEADEncryptedData data = AEAD::encrypt(prekey, "Associated data", ser); + AEADEncryptedData data = AEAD::encrypt(prekey, "Associated data", ser_message); + + // Serialize encrypted data + CDataStream ser_data(SER_NETWORK, PROTOCOL_VERSION); + ser_data << data; + + // Deserialize encrypted data + AEADEncryptedData data_deser; + ser_data >> data_deser; // Decrypt - ser = AEAD::decrypt_and_verify(prekey, "Associated data", data); + ser_message = AEAD::decrypt_and_verify(prekey, "Associated data", data_deser); // Deserialize int message_; - ser >> message_; + ser_message >> message_; BOOST_CHECK_EQUAL(message_, message); }