From bfabc979bb884f9ece09e4f2373459eb5ad42d1a Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 20 Feb 2026 01:46:15 +0900 Subject: [PATCH] fix: reject invalid markers in account_objects RPC calls (#5046) --- src/test/rpc/AccountObjects_test.cpp | 228 +++++++++++++++++++--- src/xrpld/rpc/detail/RPCHelpers.cpp | 18 +- src/xrpld/rpc/handlers/AccountObjects.cpp | 17 +- 3 files changed, 220 insertions(+), 43 deletions(-) diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 3b9f78ab9..eda0e5c9f 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -1144,35 +1144,6 @@ public: BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::account), 0)); } - void - testAccountNFTs() - { - testcase("account_nfts"); - - using namespace jtx; - Env env(*this); - - // test validation - { - auto testInvalidAccountParam = [&](auto const& param) { - Json::Value params; - params[jss::account] = param; - auto jrr = env.rpc( - "json", "account_nfts", to_string(params))[jss::result]; - BEAST_EXPECT(jrr[jss::error] == "invalidParams"); - BEAST_EXPECT( - jrr[jss::error_message] == "Invalid field 'account'."); - }; - - testInvalidAccountParam(1); - testInvalidAccountParam(1.1); - testInvalidAccountParam(true); - testInvalidAccountParam(Json::Value(Json::nullValue)); - testInvalidAccountParam(Json::Value(Json::objectValue)); - testInvalidAccountParam(Json::Value(Json::arrayValue)); - } - } - void testNFTsMarker() { @@ -1295,6 +1266,202 @@ public: "Invalid field \'marker\'.")); } + void + testAccountNFTs() + { + testcase("account_nfts"); + + using namespace jtx; + Env env(*this); + + // test validation + { + auto testInvalidAccountParam = [&](auto const& param) { + Json::Value params; + params[jss::account] = param; + auto jrr = env.rpc( + "json", "account_nfts", to_string(params))[jss::result]; + BEAST_EXPECT(jrr[jss::error] == "invalidParams"); + BEAST_EXPECT( + jrr[jss::error_message] == "Invalid field 'account'."); + }; + + testInvalidAccountParam(1); + testInvalidAccountParam(1.1); + testInvalidAccountParam(true); + testInvalidAccountParam(Json::Value(Json::nullValue)); + testInvalidAccountParam(Json::Value(Json::objectValue)); + testInvalidAccountParam(Json::Value(Json::arrayValue)); + } + } + + void + testAccountObjectMarker() + { + testcase("AccountObjectMarker"); + + using namespace jtx; + Env env(*this); + + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, bob, carol); + + unsigned const accountObjectSize = 30; + for (unsigned i = 0; i < accountObjectSize; i++) + env(check::create(alice, bob, XRP(10))); + + for (unsigned i = 0; i < 10; i++) + env(token::mint(carol, 0)); + + env.close(); + + unsigned const limit = 11; + Json::Value marker; + + // test account_objects with a limit and update marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + marker = resp[jss::result][jss::marker]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == limit); + } + + // test account_objects with valid marker and update marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = marker; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + marker = resp[jss::result][jss::marker]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == limit); + } + + // this lambda function is used to check invalid marker response. + auto testInvalidMarker = [&](std::string& marker) { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::ledger_index] = jss::validated; + params[jss::marker] = marker; + Json::Value const resp = + env.rpc("json", "account_objects", to_string(params)); + return resp[jss::result][jss::error_message] == + "Invalid field \'marker\'."; + }; + + auto const markerStr = marker.asString(); + auto const& idx = markerStr.find(','); + auto const dirIndex = markerStr.substr(0, idx); + auto const entryIndex = markerStr.substr(idx + 1); + + // test account_objects with an invalid marker that contains no ',' + { + std::string s = dirIndex + entryIndex; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test invalid marker by adding invalid string after the maker: + // "dirIndex,entryIndex,1234" + { + std::string s = markerStr + ",1234"; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test account_objects with an invalid marker containing invalid + // dirIndex by replacing some characters from the dirIndex. + { + std::string s = markerStr; + s.replace(0, 7, "FFFFFFF"); + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test account_objects with an invalid marker containing invalid + // entryIndex by replacing some characters from the entryIndex. + { + std::string s = entryIndex; + s.replace(0, 7, "FFFFFFF"); + s = dirIndex + ',' + s; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test account_objects with an invalid marker containing invalid + // dirIndex with marker: ",entryIndex" + { + std::string s = ',' + entryIndex; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test account_objects with marker: "0,entryIndex", this is still + // valid, because when dirIndex = 0, we will use root key to find + // dir. + { + std::string s = "0," + entryIndex; + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = s; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == limit); + } + + // test account_objects with an invalid marker containing invalid + // entryIndex with marker: "dirIndex," + { + std::string s = dirIndex + ','; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // test account_objects with an invalid marker containing invalid + // entryIndex with marker: "dirIndex,0" + { + std::string s = dirIndex + ",0"; + BEAST_EXPECT(testInvalidMarker(s)); + } + + // continue getting account_objects with valid marker. This will be the + // last page, so response will not contain any marker. + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = marker; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT( + accountObjects.size() == accountObjectSize - limit * 2); + BEAST_EXPECT(!resp[jss::result].isMember(jss::marker)); + } + + // test account_objects when the account only have nft pages, but + // provided invalid entry index. + { + Json::Value params; + params[jss::account] = carol.human(); + params[jss::limit] = 10; + params[jss::marker] = "0," + entryIndex; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(accountObjects.size() == 0); + } + } + void run() override { @@ -1304,8 +1471,9 @@ public: testUnsteppedThenStepped(all); testUnsteppedThenSteppedWithNFTs(all); testObjectTypes(all); - testAccountNFTs(); testNFTsMarker(); + testAccountNFTs(); + testAccountObjectMarker(); } }; diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index 988a01412..ac9e78ef9 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -161,6 +161,10 @@ getAccountObjects( std::uint32_t const limit, Json::Value& jvResult) { + // check if dirIndex is valid + if (!dirIndex.isZero() && !ledger.read({ltDIR_NODE, dirIndex})) + return false; + auto typeMatchesFilter = [](std::vector const& typeFilter, LedgerEntryType ledgerType) { auto it = std::find(typeFilter.begin(), typeFilter.end(), ledgerType); @@ -252,8 +256,18 @@ getAccountObjects( if (!dir) { // it's possible the user had nftoken pages but no - // directory entries - return mlimit < limit; + // directory entries. If there's no nftoken page, we will + // give empty array for account_objects. + if (mlimit >= limit) + jvResult[jss::account_objects] = Json::arrayValue; + + // non-zero dirIndex validity was checked in the beginning of this + // function; by this point, it should be zero. This function returns + // true regardless of nftoken page presence; if absent, account_objects + // is already set as an empty array. Notice we will only return false in + // this function when entryIndex can not be found, indicating an invalid + // marker error. + return true; } std::uint32_t i = 0; diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp index e4d588537..7aa54d615 100644 --- a/src/xrpld/rpc/handlers/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/AccountObjects.cpp @@ -272,18 +272,15 @@ doAccountObjects(RPC::JsonContext& context) if (!marker.isString()) return RPC::expected_field_error(jss::marker, "string"); - std::stringstream ss(marker.asString()); - std::string s; - if (!std::getline(ss, s, ',')) + auto const& markerStr = marker.asString(); + auto const& idx = markerStr.find(','); + if (idx == std::string::npos) return RPC::invalid_field_error(jss::marker); - if (!dirIndex.parseHex(s)) + if (!dirIndex.parseHex(markerStr.substr(0, idx))) return RPC::invalid_field_error(jss::marker); - if (!std::getline(ss, s, ',')) - return RPC::invalid_field_error(jss::marker); - - if (!entryIndex.parseHex(s)) + if (!entryIndex.parseHex(markerStr.substr(idx + 1))) return RPC::invalid_field_error(jss::marker); } @@ -295,9 +292,7 @@ doAccountObjects(RPC::JsonContext& context) entryIndex, limit, result)) - { - result[jss::account_objects] = Json::arrayValue; - } + return RPC::invalid_field_error(jss::marker); result[jss::account] = toBase58(accountID); context.loadType = Resource::feeMediumBurdenRPC;