diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp index ad3273220fd..36abab7a496 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp @@ -68,6 +68,10 @@ ConfidentialMPTClawback::preclaim(PreclaimContext const& ctx) if (!sleIssuance->isFlag(lsfMPTCanClawback)) return tecNO_PERMISSION; + // Check if issuance allows confidential transfer + if (!sleIssuance->isFlag(lsfMPTCanConfidentialAmount)) + return tecNO_PERMISSION; // LCOV_EXCL_LINE + // Check holder's MPToken auto const sleHolderMPToken = ctx.view.read(keylet::mptoken(mptIssuanceID, holder)); if (!sleHolderMPToken) @@ -75,7 +79,11 @@ ConfidentialMPTClawback::preclaim(PreclaimContext const& ctx) // Check if holder has confidential balances to claw back if (!sleHolderMPToken->isFieldPresent(sfIssuerEncryptedBalance)) - return tecNO_PERMISSION; + return tecNO_PERMISSION; // LCOV_EXCL_LINE + + // Check if Holder has ElGamal public Key + if (!sleHolderMPToken->isFieldPresent(sfHolderEncryptionKey)) + return tecNO_PERMISSION; // LCOV_EXCL_LINE // Sanity check: claw amount can not exceed confidential outstanding amount auto const amount = ctx.tx[sfMPTAmount]; diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index d2ebda7202f..78dd85f21e4 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -2887,9 +2887,143 @@ class ConfidentialTransfer_test : public beast::unit_test::suite }); } + // send when spending balance is 0 (key registered, inbox merged, but nothing converted) + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + MPTTester mptAlice(env, alice, {.holders = {bob, carol}}); + + mptAlice.create({ + .ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount, + }); + mptAlice.authorize({.account = bob}); + mptAlice.authorize({.account = carol}); + mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, carol, 50); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(carol); + + mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + + // Register keys only (amt=0) for both parties, then merge — spending stays 0. + mptAlice.convert({.account = bob, .amt = 0, .holderPubKey = mptAlice.getPubKey(bob)}); + mptAlice.mergeInbox({.account = bob}); + mptAlice.convert( + {.account = carol, .amt = 0, .holderPubKey = mptAlice.getPubKey(carol)}); + + BEAST_EXPECT( + mptAlice.getDecryptedBalance(bob, MPTTester::HOLDER_ENCRYPTED_SPENDING) == 0); + + // Trying to send any amount with 0 spending balance must fail: + // the range proof for < 0 is invalid. + mptAlice.send({ + .account = bob, + .dest = carol, + .amt = 1, + .err = tecBAD_PROOF, + }); + + BEAST_EXPECT( + mptAlice.getDecryptedBalance(bob, MPTTester::HOLDER_ENCRYPTED_SPENDING) == 0); + } + // todo: test m exceeding range, require using scala and refactor } + /* TODO: uncomment when MPT crypto supports proof generation with value 0 + * Tests verifier behavior when the send amount is 0. + * + * The equality proof library and range proof library do not + * support generating proofs for amt=0 (they require a positive witness). + * To test the VERIFIER without crashing the helper, we bypass normal proof + * generation by supplying explicit ciphertexts, commitments, and a dummy + * (all-zero) proof. The preflight has no temBAD_AMOUNT guard for + * ConfidentialMPTSend, so all validation occurs in verifySendProofs. + */ + /*void + testSendZeroAmount(FeatureBitset features) + { + testcase("Send: zero amount — equality and range proof verifier behavior"); + using namespace test::jtx; + + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + MPTTester mptAlice(env, alice, {.holders = {bob, carol}}); + + mptAlice.create({ + .ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount, + }); + mptAlice.authorize({.account = bob}); + mptAlice.authorize({.account = carol}); + mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, carol, 50); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(carol); + + mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + + mptAlice.convert({.account = bob, .amt = 100, .holderPubKey = mptAlice.getPubKey(bob)}); + mptAlice.mergeInbox({.account = bob}); + + mptAlice.convert({.account = carol, .amt = 50, .holderPubKey = mptAlice.getPubKey(carol)}); + mptAlice.mergeInbox({.account = carol}); + + Buffer const bf = generateBlindingFactor(); + + // equality proof verification for amt=0. + // Encrypt 0 under each participant's key. The amount commitment is + // getTrivialCommitment() — a valid EC point that passes preflight's + // isValidCompressedECPoint check but is not the true PC for amt=0. + // The dummy ZKProof's equality component must be rejected by + // verifyMultiCiphertextEqualityProof. + mptAlice.send({ + .account = bob, + .dest = carol, + .amt = 0, + .proof = getTrivialSendProofHex(3), + .senderEncryptedAmt = mptAlice.encryptAmount(bob, 0, bf), + .destEncryptedAmt = mptAlice.encryptAmount(carol, 0, bf), + .issuerEncryptedAmt = mptAlice.encryptAmount(alice, 0, bf), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), + .err = tecBAD_PROOF, + }); + + // range proof verification for amt=0. + // Identical construction; focuses on the bulletproof range check + // embedded in ZKProof. The range proof for amount=0 with a dummy + // (all-zero) proof must also be rejected. + Buffer const bf2 = generateBlindingFactor(); + mptAlice.send({ + .account = bob, + .dest = carol, + .amt = 0, + .proof = getTrivialSendProofHex(3), + .senderEncryptedAmt = mptAlice.encryptAmount(bob, 0, bf2), + .destEncryptedAmt = mptAlice.encryptAmount(carol, 0, bf2), + .issuerEncryptedAmt = mptAlice.encryptAmount(alice, 0, bf2), + .amountCommitment = getTrivialCommitment(), + .balanceCommitment = getTrivialCommitment(), + .err = tecBAD_PROOF, + }); + + // All rejected sends must leave balances unchanged. + BEAST_EXPECT( + mptAlice.getDecryptedBalance(bob, MPTTester::HOLDER_ENCRYPTED_SPENDING) == 100); + BEAST_EXPECT( + mptAlice.getDecryptedBalance(carol, MPTTester::HOLDER_ENCRYPTED_INBOX) == 0); + }*/ + void testDelete(FeatureBitset features) { @@ -6455,6 +6589,87 @@ class ConfidentialTransfer_test : public beast::unit_test::suite BEAST_EXPECT(bobSpendingBefore == bobSpendingAfter); } + /* This test ensures that when sending confidential tokens, the encrypted + * amounts are securely locked to the correct accounts' official public keys. + * + * Attack scenario — Encrypting the issuer's copy with the wrong key: + * A sender correctly encrypts the hidden transfer amount for themselves + * and the receiver. However, they intentionally encrypt the issuer's + * copy of the data using the wrong public key (for example, using the + * receiver's key instead of the official issuer's key). */ + void + testSendWrongIssuerPublicKey(FeatureBitset features) + { + testcase("Send: issuer ciphertext encrypted under wrong public key"); + using namespace test::jtx; + + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + MPTTester mptAlice(env, alice, {.holders = {bob, carol}}); + + mptAlice.create( + {.ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount}); + mptAlice.authorize({.account = bob}); + mptAlice.authorize({.account = carol}); + mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, carol, 50); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(carol); + + mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + + mptAlice.convert({.account = bob, .amt = 100, .holderPubKey = mptAlice.getPubKey(bob)}); + mptAlice.mergeInbox({.account = bob}); + + mptAlice.convert({.account = carol, .amt = 50, .holderPubKey = mptAlice.getPubKey(carol)}); + mptAlice.mergeInbox({.account = carol}); + + auto const bobSpendingBefore = + mptAlice.getDecryptedBalance(bob, MPTTester::HOLDER_ENCRYPTED_SPENDING); + BEAST_EXPECT(bobSpendingBefore == 100); + + // issuer ciphertext encrypted under carol's holder key + // (should be under alice's registered issuer key). + { + Buffer const bf = generateBlindingFactor(); + Buffer const wrongIssuerCt = mptAlice.encryptAmount(carol, 10, bf); + + mptAlice.send({ + .account = bob, + .dest = carol, + .amt = 10, + .issuerEncryptedAmt = wrongIssuerCt, + .err = tecBAD_PROOF, + }); + } + + // issuer ciphertext encrypted under bob's holder key + // (the sender's own key — still not the registered issuer key). + { + Buffer const bf = generateBlindingFactor(); + Buffer const wrongIssuerCt = mptAlice.encryptAmount(bob, 10, bf); + + mptAlice.send({ + .account = bob, + .dest = carol, + .amt = 10, + .issuerEncryptedAmt = wrongIssuerCt, + .err = tecBAD_PROOF, + }); + } + + // all balances unchanged + BEAST_EXPECT( + mptAlice.getDecryptedBalance(bob, MPTTester::HOLDER_ENCRYPTED_SPENDING) == + bobSpendingBefore); + BEAST_EXPECT(mptAlice.getDecryptedBalance(carol, MPTTester::HOLDER_ENCRYPTED_INBOX) == 0); + } + // Exercises every Confidential Transfer transaction type (MPTokenIssuanceSet, // Convert, MergeInbox, Send, ConvertBack) using tickets instead of regular account // sequence numbers. @@ -6684,6 +6899,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite testSendPreflight(features); testSendPreclaim(features); testSendRangeProof(features); + // testSendZeroAmount(features); testSendDepositPreauth(features); testSendCredentialValidation(features); testSendWithAuditor(features); @@ -6710,6 +6926,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite testHomomorphicCiphertextModification(features); testConvertBackHomomorphicUnderflow(features); + testSendWrongIssuerPublicKey(features); + // Replay Tests testMutatePrivacy(features); testProofContextBinding(features); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index ace5ad6449f..fed3649d1c5 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -1359,9 +1359,9 @@ MPTTester::send(MPTConfidentialSend const& arg) // Skip proof generation if encrypted balance is missing (e.g., // feature disabled), or when the sender and destination are the - // same (malformed case causing pcm to be zero). This prevents a - // crash and allows certain error cases to be tested. - if (arg.account != arg.dest && prevEncryptedSenderSpending) + // same (malformed case causing pcm to be zero), or when spending + // balance is 0 (getBalanceLinkageProof throws for zero balance). + if (arg.account != arg.dest && prevEncryptedSenderSpending && *prevSenderSpending > 0) { proof = getConfidentialSendProof( *arg.account, @@ -1384,7 +1384,7 @@ MPTTester::send(MPTConfidentialSend const& arg) jv[sfZKProof.jsonName] = strHex(*proof); else { - size_t const sizeEquality = secp256k1_mpt_prove_same_plaintext_multi_size(nRecipients); + size_t const sizeEquality = secp256k1_mpt_proof_equality_shared_r_size(nRecipients); size_t const dummySize = sizeEquality + 2 * ecPedersenProofLength + ecDoubleBulletproofLength;