From 639943123cced8fe0aff2b6477527558d31b5f6b Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:49:02 +0000 Subject: [PATCH] fix: Validate buy/sell flag in nft RPC input (#7725) --- src/test/app/NFToken_test.cpp | 82 +++++++++++++++++++ .../rpc/handlers/orderbook/NFTOffersHelpers.h | 11 +++ 2 files changed, 93 insertions(+) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index a7437eea7f..7fcd34640b 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -4790,6 +4790,87 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite checkOffers("nft_buy_offers", 501, 2, __LINE__); } + void + testNftXxxOffersMarkerWrongSide(FeatureBitset features) + { + // A pagination marker passed to nft_buy_offers / nft_sell_offers must + // reference an offer on the same side (buy vs. sell) as the directory + // being enumerated. A wrong-side marker is rejected with invalidParams. + // + // Note: the pre-fix code also returned invalidParams for a wrong-side + // marker, but only after scanning the entire target directory (an + // O(directory size) walk usable to burn CPU). The fix short-circuits + // that scan. The scan-avoidance is not observable from the RPC + // response, so this test locks the rejection contract (wrong-side -> + // error, same-side -> success) rather than the performance property. + testcase("nft_buy_offers and nft_sell_offers wrong-side marker"); + + using namespace test::jtx; + + Env env{*this, features}; + + Account const issuer{"issuer"}; + Account const buyer{"buyer"}; + + env.fund(XRP(10000), issuer, buyer); + env.close(); + + // Mint a transferable NFT. + uint256 const nftID{token::getNextID(env, issuer, 0u, tfTransferable)}; + env(token::mint(issuer, 0), Txflags(tfTransferable)); + env.close(); + + // Create one sell offer (from the issuer, who owns the NFT) and one + // buy offer (from the buyer) for the same NFT. + env(token::createOffer(issuer, nftID, XRP(100)), Txflags(tfSellNFToken)); + env(token::createOffer(buyer, nftID, XRP(50)), token::Owner(issuer)); + env.close(); + + // Grab the index of the single offer on each side from the RPC + // response so we can use it as a marker. + auto firstOfferIndex = [this, &env, &nftID](char const* request) { + json::Value params; + params[jss::nft_id] = to_string(nftID); + json::Value const result = env.rpc("json", request, to_string(params))[jss::result]; + BEAST_EXPECT(result.isMember(jss::offers) && result[jss::offers].size() == 1); + return result[jss::offers][0u][jss::nft_offer_index].asString(); + }; + + std::string const sellOfferIndex = firstOfferIndex("nft_sell_offers"); + std::string const buyOfferIndex = firstOfferIndex("nft_buy_offers"); + + auto queryWithMarker = [&env, &nftID](char const* request, std::string const& marker) { + json::Value params; + params[jss::nft_id] = to_string(nftID); + params[jss::marker] = marker; + return env.rpc("json", request, to_string(params))[jss::result]; + }; + + // A marker referencing an offer on the wrong side is rejected with + // invalidParams. + { + // Sell-side marker passed to nft_buy_offers. + json::Value const result = queryWithMarker("nft_buy_offers", sellOfferIndex); + BEAST_EXPECT(result[jss::error].asString() == "invalidParams"); + } + { + // Buy-side marker passed to nft_sell_offers. + json::Value const result = queryWithMarker("nft_sell_offers", buyOfferIndex); + BEAST_EXPECT(result[jss::error].asString() == "invalidParams"); + } + + // A same-side marker is still accepted. With a single offer on each + // side, resuming after it simply yields no further offers. + { + json::Value const result = queryWithMarker("nft_buy_offers", buyOfferIndex); + BEAST_EXPECT(!result.isMember(jss::error)); + } + { + json::Value const result = queryWithMarker("nft_sell_offers", sellOfferIndex); + BEAST_EXPECT(!result.isMember(jss::error)); + } + } + void testNFTokenNegOffer(FeatureBitset features) { @@ -7305,6 +7386,7 @@ protected: testNFTokenWithTickets(features); testNFTokenDeleteAccount(features); testNftXxxOffers(features); + testNftXxxOffersMarkerWrongSide(features); testNFTokenNegOffer(features); testIOUWithTransferFee(features); testBrokeredSaleToSelf(features); diff --git a/src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h b/src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h index e03830ae0d..21bf3f8be8 100644 --- a/src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h +++ b/src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h @@ -93,6 +93,17 @@ enumerateNFTOffers(rpc::JsonContext& context, uint256 const& nftId, Keylet const if (!sle || nftId != sle->getFieldH256(sfNFTokenID)) return rpcError(RpcInvalidParams); + // Reject a marker that references an offer on the opposite side + // (buy vs. sell) of the directory being enumerated. Without this + // check the marker's node hint points into the other directory, so + // forEachItemAfter never finds `startAfter` and instead scans every + // page of `directory` before returning invalidParams -- turning an + // O(1) rejection into an O(directory size) walk. + auto const offerDir = + sle->isFlag(lsfSellNFToken) ? keylet::nftSells(nftId) : keylet::nftBuys(nftId); + if (directory.key != offerDir.key) + return rpcError(RpcInvalidParams); + startHint = sle->getFieldU64(sfNFTokenOfferNode); appendNftOfferJson(context.app, sle, jsonOffers); offers.reserve(reserve);