From 2f97a23cda11013e157850075170ae0149f17c42 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Thu, 6 Aug 2026 13:24:58 -0400 Subject: [PATCH 1/5] fix: Add zero keylet check --- .../xrpl/ledger/helpers/CredentialHelpers.h | 3 +- .../ledger/helpers/CredentialHelpers.cpp | 24 ++++++++++- .../tx/transactors/account/AccountDelete.cpp | 2 +- .../tx/transactors/escrow/EscrowFinish.cpp | 2 +- .../tx/transactors/payment/Payment.cpp | 2 +- .../payment_channel/PaymentChannelClaim.cpp | 2 +- .../transactors/token/ConfidentialMPTSend.cpp | 2 +- src/test/app/DepositAuth_test.cpp | 42 +++++++++++++++++++ 8 files changed, 72 insertions(+), 7 deletions(-) diff --git a/include/xrpl/ledger/helpers/CredentialHelpers.h b/include/xrpl/ledger/helpers/CredentialHelpers.h index 8e78a009235..8b1c819bf48 100644 --- a/include/xrpl/ledger/helpers/CredentialHelpers.h +++ b/include/xrpl/ledger/helpers/CredentialHelpers.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -34,7 +35,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j); // Amendment and parameters checks for sfCredentialIDs field NotTEC -checkFields(STTx const& tx, beast::Journal j); +checkFields(STTx const& tx, Rules const& rules, beast::Journal j); // Accessing the ledger to check if provided credentials are valid. Do not use // in doApply (only in preclaim) since it does not remove expired credentials. diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index 226ea100e9c..af2ecefe04a 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -52,6 +53,9 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) for (auto const& h : arr) { // Credentials already checked in preclaim. Look only for expired here. + if (view.rules().enabled(fixCleanup3_4_0) && h.isZero()) + continue; // LCOV_EXCL_LINE + auto const k = keylet::credential(h); auto const sleCred = view.peek(k); @@ -124,7 +128,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j) } NotTEC -checkFields(STTx const& tx, beast::Journal j) +checkFields(STTx const& tx, Rules const& rules, beast::Journal j) { if (!tx.isFieldPresent(sfCredentialIDs)) return tesSUCCESS; @@ -137,6 +141,13 @@ checkFields(STTx const& tx, beast::Journal j) return temMALFORMED; } + if (rules.enabled(fixCleanup3_4_0) && + std::ranges::any_of(credentials, [](uint256 const& id) { return id.isZero(); })) + { + JLOG(j.trace()) << "Malformed transaction: zero credential ID."; + return temMALFORMED; + } + std::unordered_set duplicates; for (auto const& cred : credentials) { @@ -160,6 +171,14 @@ valid(STTx const& tx, ReadView const& view, AccountID const& src, beast::Journal auto const& credIDs(tx.getFieldV256(sfCredentialIDs)); for (auto const& h : credIDs) { + if (view.rules().enabled(fixCleanup3_4_0) && h.isZero()) + { + // LCOV_EXCL_START + JLOG(j.trace()) << "Zero credential ID."; + return tecBAD_CREDENTIALS; + // LCOV_EXCL_STOP + } + auto const sleCred = view.read(keylet::credential(h)); if (!sleCred) { @@ -234,6 +253,9 @@ authorizedDepositPreauth(ReadView const& view, STVector256 const& credIDs, Accou lifeExtender.reserve(credIDs.size()); for (auto const& h : credIDs) { + if (view.rules().enabled(fixCleanup3_4_0) && h.isZero()) + return tefINTERNAL; // LCOV_EXCL_LINE + auto sleCred = view.read(keylet::credential(h)); if (!sleCred) // already checked in preclaim return tefINTERNAL; // LCOV_EXCL_LINE diff --git a/src/libxrpl/tx/transactors/account/AccountDelete.cpp b/src/libxrpl/tx/transactors/account/AccountDelete.cpp index 0055fce4036..89ac884f53f 100644 --- a/src/libxrpl/tx/transactors/account/AccountDelete.cpp +++ b/src/libxrpl/tx/transactors/account/AccountDelete.cpp @@ -50,7 +50,7 @@ AccountDelete::preflight(PreflightContext const& ctx) return temDST_IS_SRC; } - if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) return err; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp index 8bc98c7aa8e..ccc44084a69 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp @@ -110,7 +110,7 @@ EscrowFinish::preflightSigValidated(PreflightContext const& ctx) } } - if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) return err; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 17c96a1919f..c8b00f01933 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -281,7 +281,7 @@ Payment::preflight(PreflightContext const& ctx) } } - if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) return err; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp index b8118bc49f9..9143a675f65 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp @@ -87,7 +87,7 @@ PaymentChannelClaim::preflight(PreflightContext const& ctx) return temBAD_SIGNATURE; } - if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) return err; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp index d121ec26341..f4c7b98c41e 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp @@ -82,7 +82,7 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx) if (hasAuditor && !isValidCiphertext(ctx.tx[sfAuditorEncryptedAmount])) return temBAD_CIPHERTEXT; - if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) return err; return tesSUCCESS; diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index c75bdeaf3a9..35129a31135 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -913,6 +914,46 @@ struct DepositPreauth_test : public beast::unit_test::Suite } } + void + testZeroCredentialID(FeatureBitset features) + { + testcase("Zero credential ID"); + + using namespace jtx; + + char const credType[] = "abcde"; + Account const issuer{"issuer"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + + Env env(*this, features); + + env.fund(XRP(5000), issuer, alice, bob); + env.close(); + + env(credentials::create(alice, issuer, credType)); + env.close(); + env(credentials::accept(alice, issuer, credType)); + env.close(); + + auto const jv = credentials::ledgerEntry(env, alice, issuer, credType); + std::string const credIdx = jv[jss::result][jss::index].asString(); + + std::string const zeroIdx(64, '0'); + + // post-fixCleanup3_4_0: a zero ID is rejected by checkFields in + // preflight; pre-fixCleanup3_4_0, it will trigger assertion, so it is not testable. + env(pay(alice, bob, XRP(100)), credentials::Ids({zeroIdx}), Ter(temMALFORMED)); + env.close(); + + env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx, zeroIdx}), Ter(temMALFORMED)); + env.close(); + + // A valid credential succeeds + env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx})); + env.close(); + } + void testCredentialsCreation() { @@ -1424,6 +1465,7 @@ struct DepositPreauth_test : public beast::unit_test::Suite testPayment(supported - featureCredentials); testPayment(supported); testCredentialsPayment(); + testZeroCredentialID(supported); testCredentialsCreation(); testExpiredCreds(); testSortingCredentials(); From c5ed63dbc5c8a54209e6dad338b52728f8a9fcc2 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 7 Aug 2026 15:06:40 -0400 Subject: [PATCH 2/5] resolve comment --- src/libxrpl/ledger/helpers/CredentialHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index af2ecefe04a..fea206ea87c 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -54,7 +54,7 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) { // Credentials already checked in preclaim. Look only for expired here. if (view.rules().enabled(fixCleanup3_4_0) && h.isZero()) - continue; // LCOV_EXCL_LINE + tecINTERNAL; // LCOV_EXCL_LINE auto const k = keylet::credential(h); auto const sleCred = view.peek(k); From 572da0b1f89cdfebe13f42cc0f335a230b659579 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 7 Aug 2026 15:38:17 -0400 Subject: [PATCH 3/5] fix return --- src/libxrpl/ledger/helpers/CredentialHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index fea206ea87c..07befbbc7d4 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -54,7 +54,7 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) { // Credentials already checked in preclaim. Look only for expired here. if (view.rules().enabled(fixCleanup3_4_0) && h.isZero()) - tecINTERNAL; // LCOV_EXCL_LINE + return std::unexpected(tecINTERNAL); // LCOV_EXCL_LINE auto const k = keylet::credential(h); auto const sleCred = view.peek(k); From bec66c2bc9dc51735ae09164d3216478759e5489 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 7 Aug 2026 15:50:07 -0400 Subject: [PATCH 4/5] clang-tidy --- src/test/app/DepositAuth_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 35129a31135..a7a553b746e 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include // IWYU pragma: keep #include From 557c6c26a931bdd6177a54ab3880e30e3da5eafb Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 7 Aug 2026 16:41:54 -0400 Subject: [PATCH 5/5] address comment --- src/libxrpl/ledger/helpers/CredentialHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index 07befbbc7d4..5ba832957d3 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -175,7 +175,7 @@ valid(STTx const& tx, ReadView const& view, AccountID const& src, beast::Journal { // LCOV_EXCL_START JLOG(j.trace()) << "Zero credential ID."; - return tecBAD_CREDENTIALS; + return tecINTERNAL; // LCOV_EXCL_STOP }