From 763dd503be465e92d3ec4bf17a13b4aa366080a8 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 29 May 2026 20:16:25 -0400 Subject: [PATCH] fix: Add zero domainID check for permissionedDomain (#7362) --- .../ledger/helpers/PermissionedDEXHelpers.cpp | 12 +++++++ .../tx/transactors/dex/OfferCreate.cpp | 6 ++++ .../tx/transactors/payment/Payment.cpp | 6 ++++ src/test/app/PermissionedDEX_test.cpp | 34 +++++++++++++++++++ 4 files changed, 58 insertions(+) diff --git a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp index 0151c5b2df..f8653f7111 100644 --- a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp +++ b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include #include #include @@ -19,6 +21,16 @@ namespace xrpl::permissioned_dex { bool accountInDomain(ReadView const& view, AccountID const& account, Domain const& domainID) { + // Avoid constructing a zero-key PermissionedDomain keylet. + // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key. + if (view.rules().enabled(fixCleanup3_2_0) && domainID == beast::kZero) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::permissioned_dex::accountInDomain : domainID is zero"); + return false; + // LCOV_EXCL_STOP + } + auto const sleDomain = view.read(keylet::permissionedDomain(domainID)); if (!sleDomain) return false; diff --git a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp index 8bf69d25c0..f982837c5e 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp @@ -94,6 +94,12 @@ OfferCreate::preflight(PreflightContext const& ctx) if (tx.isFlag(tfHybrid) && !tx.isFieldPresent(sfDomainID)) return temINVALID_FLAG; + // A zero DomainID is invalid for a PermissionedDomain ledger entry because + // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key. + if (auto const domainID = tx[~sfDomainID]; + ctx.rules.enabled(fixCleanup3_2_0) && domainID && *domainID == beast::kZero) + return temMALFORMED; + bool const bImmediateOrCancel(tx.isFlag(tfImmediateOrCancel)); bool const bFillOrKill(tx.isFlag(tfFillOrKill)); diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 1848d34786..d7ad5d3b3c 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -125,6 +125,12 @@ Payment::preflight(PreflightContext const& ctx) if (!mpTokensV2 && isDstMPT && ctx.tx.isFieldPresent(sfPaths)) return temMALFORMED; + // A zero DomainID is invalid for a PermissionedDomain ledger entry because + // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key. + if (auto const domainID = tx[~sfDomainID]; + ctx.rules.enabled(fixCleanup3_2_0) && domainID && *domainID == beast::kZero) + return temMALFORMED; + bool const partialPaymentAllowed = tx.isFlag(tfPartialPayment); bool const limitQuality = tx.isFlag(tfLimitQuality); bool const defaultPathsAllowed = !tx.isFlag(tfNoRippleDirect); diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index c6e94d7994..a88cbaa868 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -197,6 +197,20 @@ class PermissionedDEX_test : public beast::unit_test::Suite env.close(); } + // test preflight - malformed DomainID being zero + // Only test this with fixCleanup3_2_0 enabled. Without the fix, + // an assert-enabled build can crash when Ledger::read() receives + // a zero-key PermissionedDomain keylet. + if (features[fixCleanup3_2_0]) + { + Env env(*this, features); + auto const& [gw_, domainOwner, alice_, bob_, carol_, USD, domainID, credType] = + PermissionedDEX(env); + + env(offer(bob_, XRP(10), USD(10)), Domain(uint256{}), Ter(temMALFORMED)); + env.close(); + } + // preclaim - someone outside of the domain cannot create domain offer { Env env(*this, features); @@ -396,6 +410,24 @@ class PermissionedDEX_test : public beast::unit_test::Suite env.close(); } + // test preflight - malformed DomainID being zero + // Only test this with fixCleanup3_2_0 enabled. Without the fix, + // an assert-enabled build can crash when Ledger::read() receives + // a zero-key PermissionedDomain keylet. + if (features[fixCleanup3_2_0]) + { + Env env(*this, features); + auto const& [gw_, domainOwner, alice_, bob_, carol_, USD, domainID, credType] = + PermissionedDEX(env); + + env(pay(bob_, alice_, USD(10)), + Path(~USD), + Sendmax(XRP(10)), + Domain(uint256{}), + Ter(temMALFORMED)); + env.close(); + } + // preclaim - cannot send payment with non existent domain { Env env(*this, features); @@ -1772,7 +1804,9 @@ public: // Test domain offer (w/o hybrid) testOfferCreate(all); + testOfferCreate(all - fixCleanup3_2_0); testPayment(all); + testPayment(all - fixCleanup3_2_0); testBookStep(all); testRippling(all); testOfferTokenIssuerInDomain(all);