fix: Validate buy/sell flag in nft RPC input (#7725)

This commit is contained in:
Chenna Keshava B S
2026-08-11 00:49:02 +00:00
committed by GitHub
parent 909cc5bba9
commit 639943123c
2 changed files with 93 additions and 0 deletions

View File

@@ -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);

View File

@@ -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);