Add ProofReader and auth/lock and overflow edge case checks (#6651)

This commit is contained in:
Shawn Xie
2026-03-26 13:16:51 -04:00
committed by GitHub
parent 4c0e6012e3
commit b34ecc476a
6 changed files with 256 additions and 73 deletions

View File

@@ -490,4 +490,59 @@ computeSendRemainder(Slice const& balanceCommitment, Slice const& amountCommitme
*/
std::optional<Buffer>
computeConvertBackRemainder(Slice const& commitment, uint64_t amount);
/**
* @brief Sequential reader for extracting proof components from a ZKProof blob.
*
* Encapsulates the offset-based arithmetic for slicing a concatenated proof
* blob into its individual components (equality proofs, Pedersen linkage
* proofs, bulletproofs, etc.). Performs bounds checking on every read and
* tracks whether the entire blob has been consumed.
*
* Usage:
* @code
* ProofReader reader(tx[sfZKProof]);
* auto equalityProof = reader.read(sizeEquality);
* auto pedersenProof = reader.read(ecPedersenProofLength);
* if (!equalityProof || !pedersenProof || !reader.done())
* return tecINTERNAL;
* @endcode
*/
class ProofReader
{
Slice data_;
std::size_t offset_ = 0;
public:
explicit ProofReader(Slice data) : data_(data)
{
}
/**
* @brief Read the next @p length bytes from the proof blob.
*
* @param length Number of bytes to read.
* @return A Slice of the requested bytes, or std::nullopt if there are
* not enough remaining bytes.
*/
[[nodiscard]] std::optional<Slice>
read(std::size_t length)
{
if (offset_ + length > data_.size())
return std::nullopt;
auto result = data_.substr(offset_, length);
offset_ += length;
return result;
}
/**
* @brief Returns true when every byte has been consumed.
*/
[[nodiscard]] bool
done() const
{
return offset_ == data_.size();
}
};
} // namespace xrpl

View File

@@ -307,7 +307,7 @@ std::size_t constexpr permissionMaxSize = 10;
/** The maximum number of transactions that can be in a batch. */
std::size_t constexpr maxBatchTxCount = 8;
/** EC ElGamal ciphertext length 33-byte */
/** Length of one component of EC ElGamal ciphertext */
std::size_t constexpr ecGamalEncryptedLength = 33;
/** EC ElGamal ciphertext length: two 33-byte components concatenated */
@@ -331,9 +331,6 @@ std::size_t constexpr ecBlindingFactorLength = 32;
/** Length of Schnorr ZKProof for public key registration in bytes */
std::size_t constexpr ecSchnorrProofLength = 65;
/** Length of ElGamal ciphertext equality proof in bytes */
std::size_t constexpr ecCiphertextEqualityProofLength = 261;
/** Length of ElGamal Pedersen linkage proof in bytes */
std::size_t constexpr ecPedersenProofLength = 195;
@@ -347,9 +344,9 @@ std::size_t constexpr ecSingleBulletproofLength = 688;
std::size_t constexpr ecDoubleBulletproofLength = 754;
/** Compressed EC point prefix for even y-coordinate */
static constexpr std::uint8_t ecCompressedPrefixEvenY = 0x02;
std::uint8_t constexpr ecCompressedPrefixEvenY = 0x02;
/** Compressed EC point prefix for odd y-coordinate */
static constexpr std::uint8_t ecCompressedPrefixOddY = 0x03;
std::uint8_t constexpr ecCompressedPrefixOddY = 0x03;
} // namespace xrpl

View File

@@ -86,6 +86,21 @@ ConfidentialMPTConvert::preclaim(PreclaimContext const& ctx)
return tecOBJECT_NOT_FOUND;
auto const mptIssue = MPTIssue{issuanceID};
// Explicit freeze and auth checks are required because accountHolds
// with fhZERO_IF_FROZEN/ahZERO_IF_UNAUTHORIZED only implicitly rejects
// non-zero amounts. A zero-amount convert would bypass those implicit
// checks, allowing frozen or unauthorized accounts to register ElGamal
// keys and initialize confidential balance fields.
// Check lock
if (auto const ter = checkFrozen(ctx.view, account, mptIssue); !isTesSuccess(ter))
return ter;
// Check auth
if (auto const ter = requireAuth(ctx.view, mptIssue, account); !isTesSuccess(ter))
return ter;
STAmount const mptAmount =
STAmount(MPTAmount{static_cast<MPTAmount::value_type>(amount)}, mptIssue);
if (accountHolds(
@@ -119,9 +134,10 @@ ConfidentialMPTConvert::preclaim(PreclaimContext const& ctx)
getConvertContextHash(account, issuanceID, ctx.tx.getSeqProxy().value());
// when register new pk, verify through schnorr proof
if (!isTesSuccess(verifySchnorrProof(holderPubKey, ctx.tx[sfZKProof], contextHash)))
if (auto const ter = verifySchnorrProof(holderPubKey, ctx.tx[sfZKProof], contextHash);
!isTesSuccess(ter))
{
return tecBAD_PROOF;
return ter;
}
}
else
@@ -167,9 +183,12 @@ ConfidentialMPTConvert::doApply()
// Converting decreases regular balance and increases confidential outstanding.
// The confidential outstanding tracks total tokens in confidential form globally.
auto const currentCOA = (*sleIssuance)[~sfConfidentialOutstandingAmount].value_or(0);
if (amtToConvert > maxMPTokenAmount - currentCOA)
return tecINTERNAL; // LCOV_EXCL_LINE
(*sleMptoken)[sfMPTAmount] = amt - amtToConvert;
(*sleIssuance)[sfConfidentialOutstandingAmount] =
(*sleIssuance)[~sfConfidentialOutstandingAmount].value_or(0) + amtToConvert;
(*sleIssuance)[sfConfidentialOutstandingAmount] = currentCOA + amtToConvert;
Slice const holderEc = ctx_.tx[sfHolderEncryptedAmount];
Slice const issuerEc = ctx_.tx[sfIssuerEncryptedAmount];

View File

@@ -99,34 +99,19 @@ verifyProofs(
valid = false;
}
// Parse proof components using offset
auto const proof = tx[sfZKProof];
size_t remainingLength = proof.size();
size_t currentOffset = 0;
// Extract proof components
ProofReader reader(tx[sfZKProof]);
// Extract Pedersen linkage proof
if (remainingLength < ecPedersenProofLength)
return tecINTERNAL; // LCOV_EXCL_LINE
auto const pedersenProof = reader.read(ecPedersenProofLength);
auto const bulletproof = reader.read(ecSingleBulletproofLength);
auto const pedersenProof = proof.substr(currentOffset, ecPedersenProofLength);
currentOffset += ecPedersenProofLength;
remainingLength -= ecPedersenProofLength;
// Extract bulletproof
if (remainingLength < ecSingleBulletproofLength)
return tecINTERNAL; // LCOV_EXCL_LINE
auto const bulletproof = proof.substr(currentOffset, ecSingleBulletproofLength);
currentOffset += ecSingleBulletproofLength;
remainingLength -= ecSingleBulletproofLength;
if (remainingLength != 0)
if (!pedersenProof || !bulletproof || !reader.done())
return tecINTERNAL; // LCOV_EXCL_LINE
// verify el gamal pedersen linkage
if (auto const ter = verifyPcmLinkage(
PcmLinkageType::balance,
pedersenProof,
*pedersenProof,
(*mptoken)[sfConfidentialBalanceSpending],
holderPubKey,
tx[sfBalanceCommitment],
@@ -144,7 +129,8 @@ verifyProofs(
// The bulletproof verifies that the remaining balance is non-negative
std::vector<Slice> commitments{Slice(pcRem->data(), pcRem->size())};
if (auto const ter = verifyAggregatedBulletproof(bulletproof, commitments, contextHash);
if (auto const ter =
verifyAggregatedBulletproof(*bulletproof, commitments, contextHash);
!isTesSuccess(ter))
{
valid = false;

View File

@@ -80,44 +80,17 @@ verifySendProofs(
auto const hasAuditor = ctx.tx.isFieldPresent(sfAuditorEncryptedAmount);
auto const recipientCount = getConfidentialRecipientCount(hasAuditor);
auto const proof = ctx.tx[sfZKProof];
size_t remainingLength = proof.size();
size_t currentOffset = 0;
// Extract equality proof
auto const sizeEquality = getEqualityProofSize(recipientCount);
if (remainingLength < sizeEquality)
return tecINTERNAL; // LCOV_EXCL_LINE
// Extract proof components
ProofReader reader(ctx.tx[sfZKProof]);
auto const equalityProof = proof.substr(currentOffset, sizeEquality);
currentOffset += sizeEquality;
remainingLength -= sizeEquality;
auto const equalityProof = reader.read(getEqualityProofSize(recipientCount));
auto const amountLinkageProof = reader.read(ecPedersenProofLength);
auto const balanceLinkageProof = reader.read(ecPedersenProofLength);
auto const rangeProof = reader.read(ecDoubleBulletproofLength);
// Extract Pedersen linkage proof for amount commitment
if (remainingLength < ecPedersenProofLength)
return tecINTERNAL; // LCOV_EXCL_LINE
auto const amountLinkageProof = proof.substr(currentOffset, ecPedersenProofLength);
currentOffset += ecPedersenProofLength;
remainingLength -= ecPedersenProofLength;
// Extract Pedersen linkage proof for balance commitment
if (remainingLength < ecPedersenProofLength)
return tecINTERNAL; // LCOV_EXCL_LINE
auto const balanceLinkageProof = proof.substr(currentOffset, ecPedersenProofLength);
currentOffset += ecPedersenProofLength;
remainingLength -= ecPedersenProofLength;
// Extract range proof
if (remainingLength < ecDoubleBulletproofLength)
return tecINTERNAL;
auto const rangeProof = proof.substr(currentOffset, ecDoubleBulletproofLength);
currentOffset += ecDoubleBulletproofLength;
remainingLength -= ecDoubleBulletproofLength;
if (remainingLength != 0)
if (!equalityProof || !amountLinkageProof || !balanceLinkageProof || !rangeProof ||
!reader.done())
return tecINTERNAL; // LCOV_EXCL_LINE
// Prepare recipient list
@@ -150,7 +123,7 @@ verifySendProofs(
// Verify the multi-ciphertext equality proof
if (auto const ter = verifyMultiCiphertextEqualityProof(
equalityProof, recipients, recipientCount, contextHash);
*equalityProof, recipients, recipientCount, contextHash);
!isTesSuccess(ter))
{
valid = false;
@@ -159,7 +132,7 @@ verifySendProofs(
// Verify amount linkage
if (auto const ter = verifyPcmLinkage(
PcmLinkageType::amount,
amountLinkageProof,
*amountLinkageProof,
ctx.tx[sfSenderEncryptedAmount],
(*sleSenderMPToken)[sfHolderEncryptionKey],
ctx.tx[sfAmountCommitment],
@@ -172,7 +145,7 @@ verifySendProofs(
// Verify balance linkage
if (auto const ter = verifyPcmLinkage(
PcmLinkageType::balance,
balanceLinkageProof,
*balanceLinkageProof,
(*sleSenderMPToken)[sfConfidentialBalanceSpending],
(*sleSenderMPToken)[sfHolderEncryptionKey],
ctx.tx[sfBalanceCommitment],
@@ -194,7 +167,7 @@ verifySendProofs(
commitments.push_back(ctx.tx[sfAmountCommitment]);
commitments.push_back(Slice{pcRem->data(), pcRem->size()});
if (auto const ter = verifyAggregatedBulletproof(rangeProof, commitments, contextHash);
if (auto const ter = verifyAggregatedBulletproof(*rangeProof, commitments, contextHash);
!isTesSuccess(ter))
{
valid = false;

View File

@@ -1156,7 +1156,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
.account = bob,
.amt = 10,
.holderPubKey = mptAlice.getPubKey(bob),
.err = tecINSUFFICIENT_FUNDS,
.err = tecLOCKED,
});
mptAlice.set({
@@ -1211,7 +1211,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
.account = bob,
.amt = 10,
.holderPubKey = mptAlice.getPubKey(bob),
.err = tecINSUFFICIENT_FUNDS,
.err = tecNO_AUTH,
});
// auth bob
@@ -1227,6 +1227,89 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
});
}
// frozen account cannot bypass freeze check with amount=0
{
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create({
.ownerCount = 1,
.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount,
});
mptAlice.authorize({
.account = bob,
});
mptAlice.pay(alice, bob, 100);
mptAlice.generateKeyPair(alice);
mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)});
// lock bob
mptAlice.set({
.account = alice,
.holder = bob,
.flags = tfMPTLock,
});
mptAlice.generateKeyPair(bob);
// amount=0 should still be rejected when locked
mptAlice.convert({
.account = bob,
.amt = 0,
.holderPubKey = mptAlice.getPubKey(bob),
.err = tecLOCKED,
});
}
// unauthorized account cannot bypass auth check with amount=0
{
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create({
.ownerCount = 1,
.flags =
tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth | tfMPTCanConfidentialAmount,
});
mptAlice.authorize({
.account = bob,
});
mptAlice.authorize({
.account = alice,
.holder = bob,
});
mptAlice.pay(alice, bob, 100);
mptAlice.generateKeyPair(alice);
mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)});
mptAlice.generateKeyPair(bob);
// Unauthorize bob
mptAlice.authorize({
.account = alice,
.holder = bob,
.flags = tfMPTUnauthorize,
});
// amount=0 should still be rejected when unauthorized
mptAlice.convert({
.account = bob,
.amt = 0,
.holderPubKey = mptAlice.getPubKey(bob),
.err = tecNO_AUTH,
});
}
// cannot convert if auditor key is set, but auditor amount is not
// provided
{
@@ -1376,6 +1459,76 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
.err = tecBAD_PROOF,
});
}
// no holder key on ledger and no key in tx
{
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create({
.ownerCount = 1,
.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount,
});
mptAlice.authorize({
.account = bob,
});
mptAlice.pay(alice, bob, 100);
mptAlice.generateKeyPair(alice);
mptAlice.generateKeyPair(bob);
mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)});
// bob has not registered a holder key, and doesn't provide one
mptAlice.convert({
.account = bob,
.amt = 10,
.err = tecNO_PERMISSION,
});
}
// all public balance already converted, try to convert more
{
Env env{*this, features};
Account const alice("alice");
Account const bob("bob");
MPTTester mptAlice(env, alice, {.holders = {bob}});
mptAlice.create({
.ownerCount = 1,
.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanConfidentialAmount,
});
mptAlice.authorize({
.account = bob,
});
mptAlice.pay(alice, bob, 100);
mptAlice.generateKeyPair(alice);
mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)});
mptAlice.generateKeyPair(bob);
// convert entire public balance
mptAlice.convert({
.account = bob,
.amt = 100,
.holderPubKey = mptAlice.getPubKey(bob),
});
env.require(mptbalance(mptAlice, bob, 0));
// try to convert 1 more — no public balance left
mptAlice.convert({
.account = bob,
.amt = 1,
.err = tecINSUFFICIENT_FUNDS,
});
}
}
void