diff --git a/include/xrpl/protocol/ConfidentialTransfer.h b/include/xrpl/protocol/ConfidentialTransfer.h index 55a2da43fa..5bcccf2c4a 100644 --- a/include/xrpl/protocol/ConfidentialTransfer.h +++ b/include/xrpl/protocol/ConfidentialTransfer.h @@ -187,6 +187,18 @@ serializeEcPair(secp256k1_pubkey const& in1, secp256k1_pubkey const& in2, Buffer bool isValidCiphertext(Slice const& buffer); +/** + * @brief Verifies that a buffer contains a valid, parsable compressed EC point. + * + * Can be used to validate both compressed public keys and Pedersen commitments. + * Fails early if the prefix byte is not 0x02 or 0x03. + * + * @param buffer The input buffer containing a compressed EC point (33 bytes). + * @return true if the point can be parsed successfully, false otherwise. + */ +bool +isValidCompressedECPoint(Slice const& buffer); + /** * @brief Homomorphically adds two ElGamal ciphertexts. * diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index dde0eabc77..12795844c9 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -305,8 +305,11 @@ std::size_t constexpr ecGamalEncryptedTotalLength = 66; /** Length of equality ZKProof */ std::size_t constexpr ecEqualityProofLength = 98; -/** Length of EC public key */ -std::size_t constexpr ecPubKeyLength = 64; +/** Length of EC point (compressed) */ +std::size_t constexpr compressedECPointLength = 33; + +/** Length of EC public key (compressed) */ +std::size_t constexpr ecPubKeyLength = compressedECPointLength; /** Length of EC private key */ std::size_t constexpr ecPrivKeyLength = 32; @@ -323,7 +326,7 @@ std::size_t constexpr ecCiphertextEqualityProofLength = 261; /** Length of ElGamal Pedersen linkage proof */ std::size_t constexpr ecPedersenProofLength = 195; -/** Length of Pedersen Commitment proof */ -std::size_t constexpr ecPedersenCommitmentLength = 64; +/** Length of Pedersen Commitment (compressed) */ +std::size_t constexpr ecPedersenCommitmentLength = compressedECPointLength; } // namespace xrpl diff --git a/src/libxrpl/protocol/ConfidentialTransfer.cpp b/src/libxrpl/protocol/ConfidentialTransfer.cpp index 4612a41175..fd22c9f604 100644 --- a/src/libxrpl/protocol/ConfidentialTransfer.cpp +++ b/src/libxrpl/protocol/ConfidentialTransfer.cpp @@ -122,6 +122,20 @@ isValidCiphertext(Slice const& buffer) return makeEcPair(buffer, key1, key2); } +bool +isValidCompressedECPoint(Slice const& buffer) +{ + if (buffer.size() != compressedECPointLength) + return false; + + // Compressed EC points must start with 0x02 or 0x03 + if (buffer[0] != 0x02 && buffer[0] != 0x03) + return false; + + secp256k1_pubkey point; + return secp256k1_ec_pubkey_parse(secp256k1Context(), &point, buffer.data(), buffer.size()) == 1; +} + TER homomorphicAdd(Slice const& a, Slice const& b, Buffer& out) { @@ -192,8 +206,12 @@ encryptAmount(uint64_t const amt, Slice const& pubKeySlice, Slice const& blindin if (blindingFactor.size() != ecBlindingFactorLength) return std::nullopt; + if (pubKeySlice.size() != ecPubKeyLength) + return std::nullopt; + secp256k1_pubkey c1, c2, pubKey; - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return std::nullopt; if (!secp256k1_elgamal_encrypt(secp256k1Context(), &c1, &c2, &pubKey, amt, blindingFactor.data())) return std::nullopt; @@ -212,14 +230,15 @@ encryptCanonicalZeroAmount(Slice const& pubKeySlice, AccountID const& account, M return std::nullopt; // LCOV_EXCL_LINE secp256k1_pubkey c1, c2, pubKey; - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return std::nullopt; // LCOV_EXCL_LINE if (!generate_canonical_encrypted_zero(secp256k1Context(), &c1, &c2, &pubKey, account.data(), mptId.data())) - return std::nullopt; + return std::nullopt; // LCOV_EXCL_LINE Buffer buf(ecGamalEncryptedTotalLength); if (!serializeEcPair(c1, c2, buf)) - return std::nullopt; + return std::nullopt; // LCOV_EXCL_LINE return buf; } @@ -234,7 +253,8 @@ verifySchnorrProof(Slice const& pubKeySlice, Slice const& proofSlice, uint256 co return tecINTERNAL; // LCOV_EXCL_LINE secp256k1_pubkey pubKey; - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE if (secp256k1_mpt_pok_sk_verify(secp256k1Context(), proofSlice.data(), &pubKey, contextHash.data()) != 1) return tecBAD_PROOF; @@ -256,7 +276,8 @@ verifyElGamalEncryption( return tecINTERNAL; // LCOV_EXCL_LINE secp256k1_pubkey pubKey; - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE secp256k1_pubkey c1, c2; if (!makeEcPair(ciphertext, c1, c2)) @@ -339,7 +360,8 @@ verifyMultiCiphertextEqualityProof( if (recipient.publicKey.size() != ecPubKeyLength) return tecINTERNAL; // LCOV_EXCL_LINE - std::memcpy(pk[i].data, recipient.publicKey.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pk[i], recipient.publicKey.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE } int const result = secp256k1_mpt_verify_same_plaintext_multi( @@ -363,8 +385,12 @@ verifyClawbackEqualityProof( if (!makeEcPair(ciphertext, c1, c2)) return tecINTERNAL; // LCOV_EXCL_LINE + if (pubKeySlice.size() != ecPubKeyLength) + return tecINTERNAL; // LCOV_EXCL_LINE + secp256k1_pubkey pubKey; - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE // Note: c2, c1 order - the proof is generated with c2 first (the encrypted // message component) because the equality proof structure expects the @@ -413,16 +439,19 @@ verifyAmountPcmLinkage( if (!makeEcPair(encAmt, c1, c2)) return tecINTERNAL; // LCOV_EXCL_LINE - secp256k1_pubkey pubKey; if (pubKeySlice.size() != ecPubKeyLength) return tecINTERNAL; // LCOV_EXCL_LINE - secp256k1_pubkey pcm; if (pcmSlice.size() != ecPedersenCommitmentLength) return tecINTERNAL; // LCOV_EXCL_LINE - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); - std::memcpy(pcm.data, pcmSlice.data(), ecPedersenCommitmentLength); + secp256k1_pubkey pubKey; + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE + + secp256k1_pubkey pcm; + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pcm, pcmSlice.data(), ecPedersenCommitmentLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE if (secp256k1_elgamal_pedersen_link_verify( secp256k1Context(), proof.data(), &c1, &c2, &pubKey, &pcm, contextHash.data()) != 1) @@ -450,16 +479,19 @@ verifyBalancePcmLinkage( if (!makeEcPair(encAmt, c1, c2)) return tecINTERNAL; // LCOV_EXCL_LINE - secp256k1_pubkey pubKey; if (pubKeySlice.size() != ecPubKeyLength) return tecINTERNAL; // LCOV_EXCL_LINE - secp256k1_pubkey pcm; if (pcmSlice.size() != ecPedersenCommitmentLength) return tecINTERNAL; // LCOV_EXCL_LINE - std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength); - std::memcpy(pcm.data, pcmSlice.data(), ecPubKeyLength); + secp256k1_pubkey pubKey; + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE + + secp256k1_pubkey pcm; + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pcm, pcmSlice.data(), ecPedersenCommitmentLength) != 1) + return tecINTERNAL; // LCOV_EXCL_LINE // Note: c2, c1 order - the linkage proof expects the message-containing // component (c2 = m*G + r*Pk) before the blinding component (c1 = r*G). diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index 1432beee2e..f5ea8edeb8 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -48,6 +48,26 @@ class ConfidentialTransfer_test : public beast::unit_test::suite return trivialCiphertext; } + // Returns a valid compressed EC point (33 bytes) that can pass preflight + // validation but contains invalid data for preclaim test purposes. + static Buffer const& + getTrivialCommitment() + { + static Buffer const trivialCommitment = []() { + Buffer buf(ecPedersenCommitmentLength); + std::memset(buf.data(), 0, ecPedersenCommitmentLength); + + // 0x02 prefix for compressed EC point with even y-coordinate + buf.data()[0] = 0x02; + // Set last byte to make it a valid x-coordinate on the curve + buf.data()[ecPedersenCommitmentLength - 1] = 0x01; + + return buf; + }(); + + return trivialCommitment; + } + std::string getTrivialSendProofHex(size_t nRecipients) { @@ -276,6 +296,9 @@ class ConfidentialTransfer_test : public beast::unit_test::suite // Holder public key is invalid (empty buffer) mptAlice.convert({.account = bob, .amt = 10, .holderPubKey = Buffer{}, .err = temMALFORMED}); + + // Holder public key has correct length but invalid EC point data + mptAlice.convert({.account = bob, .amt = 10, .holderPubKey = Buffer(ecPubKeyLength), .err = temMALFORMED}); } // when registering holder pub key, the transaction must include a @@ -388,9 +411,12 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.generateKeyPair(alice); mptAlice.generateKeyPair(bob); - // Pub key is invalid + // Issuer pub key is invalid (empty) mptAlice.set({.account = alice, .issuerPubKey = Buffer{}, .err = temMALFORMED}); + // Issuer pub key has correct length but invalid EC point data + mptAlice.set({.account = alice, .issuerPubKey = Buffer(ecPubKeyLength), .err = temMALFORMED}); + // Auditor key is invalid length mptAlice.set( {.account = alice, @@ -398,6 +424,13 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .auditorPubKey = Buffer(10), .err = temMALFORMED}); + // Auditor key has correct length but invalid EC point data + mptAlice.set( + {.account = alice, + .issuerPubKey = mptAlice.getPubKey(alice), + .auditorPubKey = Buffer(ecPubKeyLength), + .err = temMALFORMED}); + // Cannot set auditor key without issuer key mptAlice.set({.account = alice, .auditorPubKey = mptAlice.getPubKey(alice), .err = temMALFORMED}); @@ -1135,8 +1168,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(3), .senderEncryptedAmt = Buffer(ecGamalEncryptedTotalLength), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temBAD_CIPHERTEXT}); // dest encrypted amount malformed mptAlice.send( @@ -1145,8 +1178,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(3), .destEncryptedAmt = Buffer(ecGamalEncryptedTotalLength), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temBAD_CIPHERTEXT}); // issuer encrypted amount malformed mptAlice.send( @@ -1155,8 +1188,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(3), .issuerEncryptedAmt = Buffer(ecGamalEncryptedTotalLength), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temBAD_CIPHERTEXT}); // invalid proof length @@ -1165,8 +1198,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .dest = carol, .amt = 10, .proof = std::string(10, 'A'), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temMALFORMED}); // invalid amount Pedersen commitment length @@ -1176,7 +1209,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(3), .amountCommitment = Buffer(100), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .balanceCommitment = getTrivialCommitment(), .err = temMALFORMED}); // invalid balance Pedersen commitment length @@ -1185,9 +1218,29 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .dest = carol, .amt = 10, .proof = getTrivialSendProofHex(3), - .amountCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), .balanceCommitment = Buffer(100), .err = temMALFORMED}); + + // amount Pedersen commitment has correct length but invalid EC point data + mptAlice.send( + {.account = bob, + .dest = carol, + .amt = 10, + .proof = getTrivialSendProofHex(3), + .amountCommitment = Buffer(ecPedersenCommitmentLength), + .balanceCommitment = getTrivialCommitment(), + .err = temMALFORMED}); + + // balance Pedersen commitment has correct length but invalid EC point data + mptAlice.send( + {.account = bob, + .dest = carol, + .amt = 10, + .proof = getTrivialSendProofHex(3), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .err = temMALFORMED}); } // test bad ciphertext @@ -1234,8 +1287,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(4), .auditorEncryptedAmt = Buffer(10), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temBAD_CIPHERTEXT}); // auditor encrypted amount (correct length, invalid data) @@ -1245,8 +1298,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(4), .auditorEncryptedAmt = getBadCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = temBAD_CIPHERTEXT}); } } @@ -1321,8 +1374,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite jv[sfSenderEncryptedAmount] = strHex(getTrivialCiphertext()); jv[sfDestinationEncryptedAmount] = strHex(getTrivialCiphertext()); jv[sfIssuerEncryptedAmount] = strHex(getTrivialCiphertext()); - jv[sfAmountCommitment] = strHex(Buffer(ecPedersenCommitmentLength)); - jv[sfBalanceCommitment] = strHex(Buffer(ecPedersenCommitmentLength)); + jv[sfAmountCommitment] = strHex(getTrivialCommitment()); + jv[sfBalanceCommitment] = strHex(getTrivialCommitment()); jv[sfZKProof] = getTrivialSendProofHex(3); env(jv, ter(tecOBJECT_NOT_FOUND)); @@ -1339,8 +1392,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .senderEncryptedAmt = getTrivialCiphertext(), .destEncryptedAmt = getTrivialCiphertext(), .issuerEncryptedAmt = getTrivialCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = tecNO_TARGET}); } @@ -1354,8 +1407,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .senderEncryptedAmt = getTrivialCiphertext(), .destEncryptedAmt = getTrivialCiphertext(), .issuerEncryptedAmt = getTrivialCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = tecNO_PERMISSION}); mptAlice.send( {.account = dave, @@ -1365,8 +1418,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .senderEncryptedAmt = getTrivialCiphertext(), .destEncryptedAmt = getTrivialCiphertext(), .issuerEncryptedAmt = getTrivialCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = tecNO_PERMISSION}); } @@ -1380,8 +1433,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .senderEncryptedAmt = getTrivialCiphertext(), .destEncryptedAmt = getTrivialCiphertext(), .issuerEncryptedAmt = getTrivialCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = tecOBJECT_NOT_FOUND}); } @@ -1585,8 +1638,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, .proof = getTrivialSendProofHex(4), .auditorEncryptedAmt = getTrivialCiphertext(), - .amountCommitment = Buffer(ecPedersenCommitmentLength), - .balanceCommitment = Buffer(ecPedersenCommitmentLength), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), .err = tecBAD_PROOF}); } } @@ -1862,6 +1915,13 @@ class ConfidentialTransfer_test : public beast::unit_test::suite // invalid blinding factor length mptAlice.convertBack({.account = alice, .amt = 30, .blindingFactor = Buffer{}, .err = temMALFORMED}); + // Balance commitment has correct length but invalid EC point data + mptAlice.convertBack( + {.account = bob, + .amt = 30, + .pedersenCommitment = Buffer(ecPedersenCommitmentLength), + .err = temMALFORMED}); + mptAlice.convertBack({.account = bob, .amt = 30, .holderEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT}); mptAlice.convertBack({.account = bob, .amt = 30, .issuerEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT}); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 7c7dd194c8..c25e26c465 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -698,7 +698,11 @@ MPTTester::getClawbackProof( return std::nullopt; } - std::memcpy(pk.data, pubKeyBlob.data(), ecPubKeyLength); + if (!secp256k1_ec_pubkey_parse(ctx, &pk, pubKeyBlob.data(), ecPubKeyLength)) + { + return std::nullopt; + } + Buffer proof(ecEqualityProofLength); if (secp256k1_equality_plaintext_prove( @@ -722,7 +726,8 @@ MPTTester::getSchnorrProof(Account const& account, uint256 const& ctxHash) const return std::nullopt; secp256k1_pubkey pk; - std::memcpy(pk.data, pubKey->data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pk, pubKey->data(), ecPubKeyLength) != 1) + return std::nullopt; Buffer proof(ecSchnorrProofLength); @@ -785,7 +790,8 @@ MPTTester::getConfidentialSendProof( return std::nullopt; } - std::memcpy(pk[i].data, recipient.publicKey.data(), ecPubKeyLength); + if (!secp256k1_ec_pubkey_parse(ctx, &pk[i], recipient.publicKey.data(), ecPubKeyLength)) + return std::nullopt; sr.insert(sr.end(), blindingFactor.data(), blindingFactor.data() + ecBlindingFactorLength); } @@ -839,9 +845,16 @@ MPTTester::getPedersenCommitment(std::uint64_t const amount, Buffer const& peder if (pedersenBlindingFactor.size() != ecBlindingFactorLength) Throw("Invalid blinding factor size"); - // current pedersen generation implementation fails if amount is 0 + // secp256k1_mpt_pedersen_commit doesn't handle amount 0, return a trivial + // valid commitment for test purposes if (amount == 0) - return Buffer{ecPedersenCommitmentLength}; + { + Buffer buf(ecPedersenCommitmentLength); + std::memset(buf.data(), 0, ecPedersenCommitmentLength); + buf.data()[0] = 0x02; + buf.data()[ecPedersenCommitmentLength - 1] = 0x01; + return buf; + } secp256k1_pubkey commitment; auto const ctx = secp256k1Context(); @@ -852,7 +865,16 @@ MPTTester::getPedersenCommitment(std::uint64_t const amount, Buffer const& peder Throw("Pedersen commitment generation failed"); } - return Buffer{commitment.data, ecPedersenCommitmentLength}; + // Serialize commitment to compressed format (33 bytes) + unsigned char compressedCommitment[ecPedersenCommitmentLength]; + size_t outLen = ecPedersenCommitmentLength; + if (secp256k1_ec_pubkey_serialize(ctx, compressedCommitment, &outLen, &commitment, SECP256K1_EC_COMPRESSED) != 1 || + outLen != ecPedersenCommitmentLength) + { + Throw("Pedersen commitment serialization failed"); + } + + return Buffer{compressedCommitment, ecPedersenCommitmentLength}; } Buffer @@ -1427,7 +1449,15 @@ MPTTester::generateKeyPair(Account const& account) if (!secp256k1_elgamal_generate_keypair(secp256k1Context(), privKey, &pubKey)) Throw("failed to generate key pair"); - pubKeys.insert({account.id(), Buffer{pubKey.data, ecPubKeyLength}}); + // Serialize public key to compressed format (33 bytes) + unsigned char compressedPubKey[ecPubKeyLength]; + size_t outLen = ecPubKeyLength; + if (secp256k1_ec_pubkey_serialize( + secp256k1Context(), compressedPubKey, &outLen, &pubKey, SECP256K1_EC_COMPRESSED) != 1 || + outLen != ecPubKeyLength) + Throw("failed to serialize public key"); + + pubKeys.insert({account.id(), Buffer{compressedPubKey, ecPubKeyLength}}); privKeys.insert({account.id(), Buffer{privKey, ecPrivKeyLength}}); } @@ -1752,10 +1782,12 @@ MPTTester::getAmountLinkageProof( } secp256k1_pubkey pk; - std::memcpy(pk.data, pubKey.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(ctx, &pk, pubKey.data(), ecPubKeyLength) != 1) + return Buffer(); secp256k1_pubkey pcm; - std::memcpy(pcm.data, params.pedersenCommitment.data(), ecPedersenCommitmentLength); + if (secp256k1_ec_pubkey_parse(ctx, &pcm, params.pedersenCommitment.data(), ecPedersenCommitmentLength) != 1) + return Buffer(); Buffer proof(ecPedersenProofLength); if (secp256k1_elgamal_pedersen_link_prove( @@ -1798,10 +1830,12 @@ MPTTester::getBalanceLinkageProof( } secp256k1_pubkey pk; - std::memcpy(pk.data, pubKey.data(), ecPubKeyLength); + if (secp256k1_ec_pubkey_parse(ctx, &pk, pubKey.data(), ecPubKeyLength) != 1) + return Buffer(); secp256k1_pubkey pcm; - std::memcpy(pcm.data, params.pedersenCommitment.data(), ecPedersenCommitmentLength); + if (secp256k1_ec_pubkey_parse(ctx, &pcm, params.pedersenCommitment.data(), ecPedersenCommitmentLength) != 1) + return Buffer(); Buffer proof(ecPedersenProofLength); diff --git a/src/xrpld/app/tx/detail/ConfidentialMPTConvert.cpp b/src/xrpld/app/tx/detail/ConfidentialMPTConvert.cpp index 3a626c7939..52e254d45e 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMPTConvert.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMPTConvert.cpp @@ -28,7 +28,7 @@ ConfidentialMPTConvert::preflight(PreflightContext const& ctx) if (ctx.tx.isFieldPresent(sfHolderElGamalPublicKey)) { - if (ctx.tx[sfHolderElGamalPublicKey].length() != ecPubKeyLength) + if (!isValidCompressedECPoint(ctx.tx[sfHolderElGamalPublicKey])) return temMALFORMED; // proof of knowledge of the secret key corresponding to the provided diff --git a/src/xrpld/app/tx/detail/ConfidentialMPTConvertBack.cpp b/src/xrpld/app/tx/detail/ConfidentialMPTConvertBack.cpp index fa1c63bb2a..89b7b25046 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMPTConvertBack.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMPTConvertBack.cpp @@ -28,7 +28,7 @@ ConfidentialMPTConvertBack::preflight(PreflightContext const& ctx) if (ctx.tx[sfBlindingFactor].size() != ecBlindingFactorLength) return temMALFORMED; - if (ctx.tx[sfBalanceCommitment].size() != ecPedersenCommitmentLength) + if (!isValidCompressedECPoint(ctx.tx[sfBalanceCommitment])) return temMALFORMED; // check encrypted amount format after the above basic checks diff --git a/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp b/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp index f740cac1dc..a1afb26d16 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp @@ -47,9 +47,8 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx) if (ctx.tx[sfZKProof].length() != sizeEquality + sizePedersenLinkage) return temMALFORMED; - // Check the length of Pedersen commitments - if (ctx.tx[sfBalanceCommitment].size() != ecPedersenCommitmentLength || - ctx.tx[sfAmountCommitment].size() != ecPedersenCommitmentLength) + // Check the Pedersen commitments are valid + if (!isValidCompressedECPoint(ctx.tx[sfBalanceCommitment]) || !isValidCompressedECPoint(ctx.tx[sfAmountCommitment])) return temMALFORMED; // Check the encrypted amount formats, this is more expensive so put it at diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index 68d6e8597b..885ea8d0dc 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -127,10 +128,10 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) if (hasAuditorElGamalKey && !hasIssuerElGamalKey) return temMALFORMED; - if (hasIssuerElGamalKey && ctx.tx[sfIssuerElGamalPublicKey].length() != ecPubKeyLength) + if (hasIssuerElGamalKey && !isValidCompressedECPoint(ctx.tx[sfIssuerElGamalPublicKey])) return temMALFORMED; - if (hasAuditorElGamalKey && ctx.tx[sfAuditorElGamalPublicKey].length() != ecPubKeyLength) + if (hasAuditorElGamalKey && !isValidCompressedECPoint(ctx.tx[sfAuditorElGamalPublicKey])) return temMALFORMED; return tesSUCCESS;