ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 11:15:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: reject invalid markers in account_objects RPC calls (#5046)

Browse Source
This commit is contained in:
yinyiqian1
2024-10-29 16:13:01 -04:00
committed by GitHub
parent ab5d450d3c
commit d4a5f8390e
3 changed files with 190 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

168
src/test/rpc/AccountObjects_test.cpp
View File

@@ -1220,6 +1220,173 @@ public:
}
}
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
{
@@ -1229,6 +1396,7 @@ public:
testObjectTypes();
testNFTsMarker();
testAccountNFTs();
testAccountObjectMarker();
}
};

18
src/xrpld/rpc/detail/RPCHelpers.cpp
View File

@@ -158,6 +158,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<LedgerEntryType> const& typeFilter,
LedgerEntryType ledgerType) {
auto it = std::find(typeFilter.begin(), typeFilter.end(), ledgerType);
@@ -249,8 +253,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;

17
src/xrpld/rpc/handlers/AccountObjects.cpp
View File

@@ -270,18 +270,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);
}
@@ -293,9 +290,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;