Add size checks to coin deserialization

firoorg · Dec 5, 2023 · 4512bb7 · 4512bb7
1 parent 39c41e5
commit 4512bb7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 0 deletions.
diff --git a/src/libspark/aead.h b/src/libspark/aead.h
@@ -15,8 +15,20 @@ struct AEADEncryptedData {
 	template <typename Stream, typename Operation>
     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
@@ -108,11 +108,28 @@ class Coin {
 	ADD_SERIALIZE_METHODS;
 	template <typename Stream, typename Operation>
 	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
@@ -36,6 +36,9 @@ uint64_t SparkUtils::diversifier_decrypt(const std::vector<unsigned char>& 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<unsigned char> iv;
     iv.resize(AES_BLOCKSIZE);