Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix account_objects to disallow filtering by types that an account ca… #5056

Merged
merged 6 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions src/test/rpc/AccountObjects_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,19 +579,35 @@ class AccountObjects_test : public beast::unit_test::suite
(resp[jss::result][jss::account_objects].size() == size);
};

// Make a lambda that checks if the response has error for invalid type
auto acct_objs_type_is_invalid = [](Json::Value const& resp) {
yinyiqian1 marked this conversation as resolved.
Show resolved Hide resolved
return resp[jss::result].isMember(jss::error) &&
resp[jss::result][jss::error_message] ==
"Invalid field \'type\'.";
};

// Make a lambda that checks if different filtering type is invalid
auto check_invalid_filter_types = [&](AccountID const& acct) {
// we expect invalid field type reported for the following types
if (!acct_objs_type_is_invalid(acct_objs(acct, jss::amendments)) ||
!acct_objs_type_is_invalid(acct_objs(acct, jss::directory)) ||
!acct_objs_type_is_invalid(acct_objs(acct, jss::fee)) ||
!acct_objs_type_is_invalid(acct_objs(acct, jss::hashes)) ||
!acct_objs_type_is_invalid(acct_objs(acct, jss::NegativeUNL)))
return false;

return true;
};
scottschurr marked this conversation as resolved.
Show resolved Hide resolved

env.fund(XRP(10000), gw, alice);
env.close();

// Since the account is empty now, all account objects should come
// back empty.
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::account), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amendments), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::check), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::deposit_preauth), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::directory), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::escrow), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::fee), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::nft_page), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::offer), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::payment_channel), 0));
Expand All @@ -601,6 +617,9 @@ class AccountObjects_test : public beast::unit_test::suite
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::did), 0));

// invalid filter type should get invalid type response
BEAST_EXPECT(check_invalid_filter_types(gw));

// gw mints an NFT so we can find it.
uint256 const nftID{token::getNextID(env, gw, 0u, tfTransferable)};
env(token::mint(gw, 0u), txflags(tfTransferable));
Expand Down Expand Up @@ -1016,6 +1035,10 @@ class AccountObjects_test : public beast::unit_test::suite
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0));
}

// check invalid filter type again which should get invalid type
// response
BEAST_EXPECT(check_invalid_filter_types(gw));

// Run up the number of directory entries so gw has two
// directory nodes.
for (int d = 1'000'032; d >= 1'000'000; --d)
Expand All @@ -1026,10 +1049,6 @@ class AccountObjects_test : public beast::unit_test::suite

// Verify that the non-returning types still don't return anything.
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::account), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amendments), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::directory), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::fee), 0));
BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0));
}

void
Expand Down
16 changes: 16 additions & 0 deletions src/xrpld/rpc/detail/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,22 @@ chooseLedgerEntryType(Json::Value const& params)
return result;
}

bool
isAccountObjectsValidType(LedgerEntryType const& type)
{
switch (type)
{
case LedgerEntryType::ltAMENDMENTS:
case LedgerEntryType::ltDIR_NODE:
yinyiqian1 marked this conversation as resolved.
Show resolved Hide resolved
case LedgerEntryType::ltFEE_SETTINGS:
case LedgerEntryType::ltLEDGER_HASHES:
case LedgerEntryType::ltNEGATIVE_UNL:
return false;
default:
return true;
}
}

beast::SemanticVersion const firstVersion("1.0.0");
beast::SemanticVersion const goodVersion("1.0.0");
beast::SemanticVersion const lastVersion("1.0.0");
Expand Down
9 changes: 9 additions & 0 deletions src/xrpld/rpc/detail/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,15 @@ setVersion(Object& parent, unsigned int apiVersion, bool betaEnabled)
std::pair<RPC::Status, LedgerEntryType>
chooseLedgerEntryType(Json::Value const& params);

/**
* Check if the type is a valid filtering type for account_objects method
*
* Since Amendments, DirectoryNode, FeeSettings, LedgerHashes can not be
* owned by an account, this function will return false in these situations.
*/
bool
isAccountObjectsValidType(LedgerEntryType const& type);

/**
* Retrieve the api version number from the json value
*
Expand Down
3 changes: 3 additions & 0 deletions src/xrpld/rpc/handlers/AccountObjects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ doAccountObjects(RPC::JsonContext& context)
{
auto [rpcStatus, type] = RPC::chooseLedgerEntryType(params);

if (!RPC::isAccountObjectsValidType(type))
yinyiqian1 marked this conversation as resolved.
Show resolved Hide resolved
return RPC::invalid_field_error(jss::type);

if (rpcStatus)
{
result.clear();
Expand Down
Loading