ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enforce account RPC limits by account objects traversed

Browse Source
This commit is contained in:
natenichols
2021-12-15 18:56:49 -06:00
committed by manojsdoshi
parent 915fe31274
commit e28989638d
9 changed files with 358 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
src/ripple/rpc/handlers/AccountChannels.cpp
View File

@@ -101,6 +101,9 @@ doAccountChannels(RPC::JsonContext& context)
if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context))
return *err;
if (limit == 0u)
return rpcError(rpcINVALID_PARAMS);
Json::Value jsonChannels{Json::arrayValue};
struct VisitData
{
@@ -110,71 +113,93 @@ doAccountChannels(RPC::JsonContext& context)
AccountID const& raDstAccount;
};
VisitData visitData = {{}, accountID, hasDst, raDstAccount};
visitData.items.reserve(limit + 1);
uint256 startAfter;
std::uint64_t startHint;
visitData.items.reserve(limit);
uint256 startAfter = beast::zero;
std::uint64_t startHint = 0;
if (params.isMember(jss::marker))
{
Json::Value const& marker(params[jss::marker]);
if (!marker.isString())
if (!params[jss::marker].isString())
return RPC::expected_field_error(jss::marker, "string");
if (!startAfter.parseHex(marker.asString()))
// Marker is composed of a comma separated index and start hint. The
// former will be read as hex, and the latter using boost lexical cast.
std::stringstream marker(params[jss::marker].asString());
std::string value;
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
if (!startAfter.parseHex(value))
return rpcError(rpcINVALID_PARAMS);
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
try
{
startHint = boost::lexical_cast<std::uint64_t>(value);
}
catch (boost::bad_lexical_cast&)
{
return rpcError(rpcINVALID_PARAMS);
}
auto const sleChannel = ledger->read({ltPAYCHAN, startAfter});
// We then must check if the object pointed to by the marker is actually
// owned by the account in the request.
auto const sle = ledger->read({ltANY, startAfter});
if (!sleChannel)
if (!sle)
return rpcError(rpcINVALID_PARAMS);
if (!visitData.hasDst ||
visitData.raDstAccount == (*sleChannel)[sfDestination])
{
visitData.items.emplace_back(sleChannel);
startHint = sleChannel->getFieldU64(sfOwnerNode);
}
else
{
if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}
}
else
{
startHint = 0;
}
auto count = 0;
std::optional<uint256> marker = {};
std::uint64_t nextHint = 0;
if (!forEachItemAfter(
*ledger,
accountID,
startAfter,
startHint,
limit - visitData.items.size() + 1,
[&visitData, &accountID](std::shared_ptr<SLE const> const& sleCur) {
if (sleCur && sleCur->getType() == ltPAYCHAN &&
limit + 1,
[&visitData, &accountID, &count, &limit, &marker, &nextHint](
std::shared_ptr<SLE const> const& sleCur) {
if (!sleCur)
{
assert(false);
return false;
}
if (++count == limit)
{
marker = sleCur->key();
nextHint = RPC::getStartHint(sleCur, visitData.accountID);
}
if (count <= limit && sleCur->getType() == ltPAYCHAN &&
(*sleCur)[sfAccount] == accountID &&
(!visitData.hasDst ||
visitData.raDstAccount == (*sleCur)[sfDestination]))
{
visitData.items.emplace_back(sleCur);
return true;
}
return false;
return true;
}))
{
return rpcError(rpcINVALID_PARAMS);
}
if (visitData.items.size() == limit + 1)
// Both conditions need to be checked because marker is set on the limit-th
// item, but if there is no item on the limit + 1 iteration, then there is
// no need to return a marker.
if (count == limit + 1 && marker)
{
result[jss::limit] = limit;
result[jss::marker] = to_string(visitData.items.back()->key());
visitData.items.pop_back();
result[jss::marker] =
to_string(*marker) + "," + std::to_string(nextHint);
}
result[jss::account] = context.app.accountIDCache().toBase58(accountID);

135
src/ripple/rpc/handlers/AccountLines.cpp
View File

@@ -130,6 +130,9 @@ doAccountLines(RPC::JsonContext& context)
if (auto err = readLimitField(limit, RPC::Tuning::accountLines, context))
return *err;
if (limit == 0)
return rpcError(rpcINVALID_PARAMS);
// this flag allows the requester to ask incoming trustlines in default
// state be omitted
bool ignoreDefault = params.isMember(jss::ignore_default) &&
@@ -138,75 +141,59 @@ doAccountLines(RPC::JsonContext& context)
Json::Value& jsonLines(result[jss::lines] = Json::arrayValue);
VisitData visitData = {
{}, accountID, hasPeer, raPeerAccount, ignoreDefault, 0, nullptr};
unsigned int reserve(limit);
uint256 startAfter;
std::uint64_t startHint;
uint256 startAfter = beast::zero;
std::uint64_t startHint = 0;
if (params.isMember(jss::marker))
{
// We have a start point. Use limit - 1 from the result and use the
// very last one for the resume.
Json::Value const& marker(params[jss::marker]);
if (!marker.isString())
if (!params[jss::marker].isString())
return RPC::expected_field_error(jss::marker, "string");
if (!startAfter.parseHex(marker.asString()))
// Marker is composed of a comma separated index and start hint. The
// former will be read as hex, and the latter using boost lexical cast.
std::stringstream marker(params[jss::marker].asString());
std::string value;
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
auto const sleLine = ledger->read({ltRIPPLE_STATE, startAfter});
if (!sleLine)
if (!startAfter.parseHex(value))
return rpcError(rpcINVALID_PARAMS);
bool isDefault = false;
if (sleLine->getFieldAmount(sfLowLimit).getIssuer() == accountID)
{
startHint = sleLine->getFieldU64(sfLowNode);
isDefault = !(sleLine->getFieldU32(sfFlags) & lsfLowReserve);
}
else if (sleLine->getFieldAmount(sfHighLimit).getIssuer() == accountID)
{
startHint = sleLine->getFieldU64(sfHighNode);
isDefault = !(sleLine->getFieldU32(sfFlags) & lsfHighReserve);
}
else
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
// Caller provided the first line (startAfter), add it as first result
// (but only if it meets inclusion criteria)
if (isDefault && ignoreDefault)
try
{
// even though we're starting our search here we don't include the
// first entry in this edge case
visitData.items.reserve(++reserve);
startHint = boost::lexical_cast<std::uint64_t>(value);
}
else
catch (boost::bad_lexical_cast&)
{
auto const line = RippleState::makeItem(accountID, sleLine);
if (line == nullptr)
return rpcError(rpcINVALID_PARAMS);
addLine(jsonLines, *line);
visitData.items.reserve(reserve);
return rpcError(rpcINVALID_PARAMS);
}
}
else
{
startHint = 0;
// We have no start point, limit should be one higher than requested.
visitData.items.reserve(++reserve);
// We then must check if the object pointed to by the marker is actually
// owned by the account in the request.
auto const sle = ledger->read({ltANY, startAfter});
if (!sle)
return rpcError(rpcINVALID_PARAMS);
if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}
auto count = 0;
std::optional<uint256> marker = {};
std::uint64_t nextHint = 0;
{
if (!forEachItemAfter(
*ledger,
accountID,
startAfter,
startHint,
reserve,
[&visitData](std::shared_ptr<SLE const> const& sleCur) {
limit + 1,
[&visitData, &count, &marker, &limit, &nextHint](
std::shared_ptr<SLE const> const& sleCur) {
bool ignore = false;
if (visitData.ignoreDefault)
{
@@ -219,42 +206,48 @@ doAccountLines(RPC::JsonContext& context)
sleCur->getFieldU32(sfFlags) & lsfHighReserve);
}
auto const line =
RippleState::makeItem(visitData.accountID, sleCur);
if (line != nullptr &&
(!visitData.hasPeer ||
visitData.raPeerAccount == line->getAccountIDPeer()))
if (!sleCur)
{
if (!ignore)
visitData.items.emplace_back(line);
visitData.lastFound = line;
visitData.foundCount++;
return true;
assert(false);
return false;
}
return false;
if (++count == limit)
{
marker = sleCur->key();
nextHint =
RPC::getStartHint(sleCur, visitData.accountID);
}
if (!ignore && count <= limit)
{
auto const line =
RippleState::makeItem(visitData.accountID, sleCur);
if (line != nullptr &&
(!visitData.hasPeer ||
visitData.raPeerAccount ==
line->getAccountIDPeer()))
{
visitData.items.emplace_back(line);
}
}
return true;
}))
{
return rpcError(rpcINVALID_PARAMS);
}
}
// RH Note:
// If ignore_default flag is present all lines must still be iterated, the
// flag only suppresses output. It does not change how iteration works. This
// means the RPC call may return an empty set AND a marker. In this case
// another query must be made until iteration is complete if a complete set
// of non-default state lines are required.
if (visitData.items.size() == reserve || visitData.foundCount >= reserve)
// Both conditions need to be checked because marker is set on the limit-th
// item, but if there is no item on the limit + 1 iteration, then there is
// no need to return a marker.
if (count == limit + 1 && marker)
{
result[jss::limit] = limit;
RippleState::pointer line(visitData.lastFound);
result[jss::marker] = to_string(line->key());
if (visitData.items.back() == visitData.lastFound)
visitData.items.pop_back();
result[jss::marker] =
to_string(*marker) + "," + std::to_string(nextHint);
}
result[jss::account] = context.app.accountIDCache().toBase58(accountID);

88
src/ripple/rpc/handlers/AccountOffers.cpp
View File

@@ -86,68 +86,94 @@ doAccountOffers(RPC::JsonContext& context)
if (auto err = readLimitField(limit, RPC::Tuning::accountOffers, context))
return *err;
if (limit == 0)
return rpcError(rpcINVALID_PARAMS);
Json::Value& jsonOffers(result[jss::offers] = Json::arrayValue);
std::vector<std::shared_ptr<SLE const>> offers;
unsigned int reserve(limit);
uint256 startAfter;
std::uint64_t startHint;
uint256 startAfter = beast::zero;
std::uint64_t startHint = 0;
if (params.isMember(jss::marker))
{
// We have a start point. Use limit - 1 from the result and use the
// very last one for the resume.
Json::Value const& marker(params[jss::marker]);
if (!marker.isString())
if (!params[jss::marker].isString())
return RPC::expected_field_error(jss::marker, "string");
if (!startAfter.parseHex(marker.asString()))
// Marker is composed of a comma separated index and start hint. The
// former will be read as hex, and the latter using boost lexical cast.
std::stringstream marker(params[jss::marker].asString());
std::string value;
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
auto const sleOffer = ledger->read({ltOFFER, startAfter});
if (!startAfter.parseHex(value))
return rpcError(rpcINVALID_PARAMS);
if (!sleOffer || accountID != sleOffer->getAccountID(sfAccount))
if (!std::getline(marker, value, ','))
return rpcError(rpcINVALID_PARAMS);
try
{
startHint = boost::lexical_cast<std::uint64_t>(value);
}
catch (boost::bad_lexical_cast&)
{
return rpcError(rpcINVALID_PARAMS);
}
startHint = sleOffer->getFieldU64(sfOwnerNode);
// Caller provided the first offer (startAfter), add it as first result
appendOfferJson(sleOffer, jsonOffers);
offers.reserve(reserve);
}
else
{
startHint = 0;
// We have no start point, limit should be one higher than requested.
offers.reserve(++reserve);
// We then must check if the object pointed to by the marker is actually
// owned by the account in the request.
auto const sle = ledger->read({ltANY, startAfter});
if (!sle)
return rpcError(rpcINVALID_PARAMS);
if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}
auto count = 0;
std::optional<uint256> marker = {};
std::uint64_t nextHint = 0;
if (!forEachItemAfter(
*ledger,
accountID,
startAfter,
startHint,
reserve,
[&offers](std::shared_ptr<SLE const> const& offer) {
if (offer->getType() == ltOFFER)
limit + 1,
[&offers, &count, &marker, &limit, &nextHint, &accountID](
std::shared_ptr<SLE const> const& sle) {
if (!sle)
{
offers.emplace_back(offer);
return true;
assert(false);
return false;
}
return false;
if (++count == limit)
{
marker = sle->key();
nextHint = RPC::getStartHint(sle, accountID);
}
if (count <= limit && sle->getType() == ltOFFER)
{
offers.emplace_back(sle);
}
return true;
}))
{
return rpcError(rpcINVALID_PARAMS);
}
if (offers.size() == reserve)
// Both conditions need to be checked because marker is set on the limit-th
// item, but if there is no item on the limit + 1 iteration, then there is
// no need to return a marker.
if (count == limit + 1 && marker)
{
result[jss::limit] = limit;
result[jss::marker] = to_string(offers.back()->key());
offers.pop_back();
result[jss::marker] =
to_string(*marker) + "," + std::to_string(nextHint);
}
for (auto const& offer : offers)

62
src/ripple/rpc/impl/RPCHelpers.cpp
View File

@@ -20,6 +20,7 @@
#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/ledger/OpenLedger.h>
#include <ripple/app/misc/Transaction.h>
#include <ripple/app/paths/RippleState.h>
#include <ripple/app/rdb/RelationalDBInterface.h>
#include <ripple/ledger/View.h>
#include <ripple/net/RPCErr.h>
@@ -89,6 +90,47 @@ accountFromString(AccountID& result, std::string const& strIdent, bool bStrict)
return Json::objectValue;
}
std::uint64_t
getStartHint(std::shared_ptr<SLE const> const& sle, AccountID const& accountID)
{
if (sle->getType() == ltRIPPLE_STATE)
{
if (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID)
return sle->getFieldU64(sfLowNode);
else if (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID)
return sle->getFieldU64(sfHighNode);
}
if (!sle->isFieldPresent(sfOwnerNode))
return 0;
return sle->getFieldU64(sfOwnerNode);
}
bool
isOwnedByAccount(
ReadView const& ledger,
std::shared_ptr<SLE const> const& sle,
AccountID const& accountID)
{
if (sle->getType() == ltRIPPLE_STATE)
{
return (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) ||
(sle->getFieldAmount(sfHighLimit).getIssuer() == accountID);
}
else if (sle->isFieldPresent(sfAccount))
{
return sle->getAccountID(sfAccount) == accountID;
}
else if (sle->getType() == ltSIGNER_LIST)
{
Keylet const accountSignerList = keylet::signers(accountID);
return sle->key() == accountSignerList.key;
}
return false;
}
bool
getAccountObjects(
ReadView const& ledger,
@@ -144,19 +186,19 @@ getAccountObjects(
typeMatchesFilter(typeFilter.value(), sleNode->getType()))
{
jvObjects.append(sleNode->getJson(JsonOptions::none));
}
if (++i == limit)
if (++i == limit)
{
if (++iter != entries.end())
{
if (++iter != entries.end())
{
jvResult[jss::limit] = limit;
jvResult[jss::marker] =
to_string(dirIndex) + ',' + to_string(*iter);
return true;
}
break;
jvResult[jss::limit] = limit;
jvResult[jss::marker] =
to_string(dirIndex) + ',' + to_string(*iter);
return true;
}
break;
}
}

19
src/ripple/rpc/impl/RPCHelpers.h
View File

@@ -71,6 +71,25 @@ accountFromStringWithCode(
std::string const& strIdent,
bool bStrict = false);
/** Gets the start hint for traversing account objects
* @param sle - Ledger entry defined by the marker passed into the RPC.
* @param accountID - The ID of the account whose objects you are traversing.
*/
std::uint64_t
getStartHint(std::shared_ptr<SLE const> const& sle, AccountID const& accountID);
/**
* Tests if a SLE is owned by accountID.
* @param ledger - The ledger used to search for the sle.
* @param sle - The SLE to test for ownership.
* @param account - The account being tested for SLE ownership.
*/
bool
isOwnedByAccount(
ReadView const& ledger,
std::shared_ptr<SLE const> const& sle,
AccountID const& accountID);
/** Gathers all objects for an account in a ledger.
@param ledger Ledger to search account objects.
@param account AccountID to find objects for.

2
src/ripple/rpc/impl/Tuning.h
View File

@@ -46,7 +46,7 @@ static LimitRange constexpr accountObjects = {10, 200, 400};
static LimitRange constexpr accountOffers = {10, 200, 400};
/** Limits for the book_offers command. */
static LimitRange constexpr bookOffers = {0, 300, 400};
static LimitRange constexpr bookOffers = {0, 60, 100};
/** Limits for the no_ripple_check command. */
static LimitRange constexpr noRippleCheck = {10, 300, 400};

3
src/test/app/PayChan_test.cpp
View File

@@ -1033,8 +1033,7 @@ struct PayChan_test : public beast::unit_test::suite
{
// degenerate case
auto const r = testLimit(env, alice, 0);
BEAST_EXPECT(r.isMember(jss::marker));
BEAST_EXPECT(r[jss::channels].size() == 0);
BEAST_EXPECT(r.isMember(jss::error_message));
}
}

98
src/test/rpc/AccountLinesRPC_test.cpp
View File

@@ -320,7 +320,7 @@ public:
"account_lines",
R"({"account": ")" + alice.human() +
R"(", )"
R"("limit": 1, )"
R"("limit": 10, )"
R"("peer": ")" +
gw2.human() + R"("})");
auto const& line = lines[jss::result][jss::lines][0u];
@@ -363,6 +363,91 @@ public:
}
}
void
testAccountLinesMarker()
{
testcase("Entry pointed to by marker is not owned by account");
using namespace test::jtx;
Env env(*this);
// The goal of this test is observe account_lines RPC calls return an
// error message when the SLE pointed to by the marker is not owned by
// the Account being traversed.
//
// To start, we'll create an environment with some trust lines, offers
// and a signers list.
Account const alice{"alice"};
Account const becky{"becky"};
Account const gw1{"gw1"};
env.fund(XRP(10000), alice, becky, gw1);
env.close();
// Give alice a SignerList.
Account const bogie{"bogie"};
env(signers(alice, 2, {{bogie, 3}}));
env.close();
auto const EUR = gw1["EUR"];
env(trust(alice, EUR(200)));
env(trust(becky, EUR(200)));
env.close();
// Get all account objects for alice and verify that her
// signerlist is first. This is only a (reliable) coincidence of
// object naming. So if any of alice's objects are renamed this
// may fail.
Json::Value const aliceObjects = env.rpc(
"json",
"account_objects",
R"({"account": ")" + alice.human() +
R"(", )"
R"("limit": 10})");
Json::Value const& aliceSignerList =
aliceObjects[jss::result][jss::account_objects][0u];
if (!(aliceSignerList[sfLedgerEntryType.jsonName] == jss::SignerList))
{
fail(
"alice's account objects are misordered. "
"Please reorder the objects so the SignerList is first.",
__FILE__,
__LINE__);
return;
}
// Get account_lines for alice. Limit at 1, so we get a marker
// pointing to her SignerList.
auto const aliceLines1 = env.rpc(
"json",
"account_lines",
R"({"account": ")" + alice.human() + R"(", "limit": 1})");
BEAST_EXPECT(aliceLines1[jss::result].isMember(jss::marker));
// Verify that the marker points at the signer list.
std::string const aliceMarker =
aliceLines1[jss::result][jss::marker].asString();
std::string const markerIndex =
aliceMarker.substr(0, aliceMarker.find(','));
BEAST_EXPECT(markerIndex == aliceSignerList[jss::index].asString());
// When we fetch Alice's remaining lines we should find one and no more.
auto const aliceLines2 = env.rpc(
"json",
"account_lines",
R"({"account": ")" + alice.human() + R"(", "marker": ")" +
aliceMarker + R"("})");
BEAST_EXPECT(aliceLines2[jss::result][jss::lines].size() == 1);
BEAST_EXPECT(!aliceLines2[jss::result].isMember(jss::marker));
// Get account lines for beckys account, using alices SignerList as a
// marker. This should cause an error.
auto const beckyLines = env.rpc(
"json",
"account_lines",
R"({"account": ")" + becky.human() + R"(", "marker": ")" +
aliceMarker + R"("})");
BEAST_EXPECT(beckyLines[jss::result].isMember(jss::error_message));
}
void
testAccountLineDelete()
{
@@ -390,8 +475,10 @@ public:
env.close();
auto const USD = gw1["USD"];
auto const AUD = gw1["AUD"];
auto const EUR = gw2["EUR"];
env(trust(alice, USD(200)));
env(trust(alice, AUD(200)));
env(trust(becky, EUR(200)));
env(trust(cheri, EUR(200)));
env.close();
@@ -414,7 +501,7 @@ public:
"account_lines",
R"({"account": ")" + alice.human() +
R"(", )"
R"("limit": 1})");
R"("limit": 2})");
BEAST_EXPECT(
linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD");
BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker));
@@ -966,7 +1053,7 @@ public:
R"({"account": ")" +
alice.human() +
R"(", )"
R"("limit": 1, )"
R"("limit": 10, )"
R"("peer": ")" +
gw2.human() + R"("}})");
auto const& line = lines[jss::result][jss::lines][0u];
@@ -1068,8 +1155,10 @@ public:
env.close();
auto const USD = gw1["USD"];
auto const AUD = gw1["AUD"];
auto const EUR = gw2["EUR"];
env(trust(alice, USD(200)));
env(trust(alice, AUD(200)));
env(trust(becky, EUR(200)));
env(trust(cheri, EUR(200)));
env.close();
@@ -1098,7 +1187,7 @@ public:
R"({"account": ")" +
alice.human() +
R"(", )"
R"("limit": 1}})");
R"("limit": 2}})");
BEAST_EXPECT(
linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD");
BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker));
@@ -1143,6 +1232,7 @@ public:
run() override
{
testAccountLines();
testAccountLinesMarker();
testAccountLineDelete();
testAccountLines2();
testAccountLineDelete2();

18
src/test/rpc/AccountOffers_test.cpp
View File

@@ -81,7 +81,8 @@ public:
"json", "account_offers", jvParams.toStyledString())[jss::result];
auto const& jro_l = jrr_l[jss::offers];
BEAST_EXPECT(checkMarker(jrr_l));
BEAST_EXPECT(checkArraySize(jro_l, 10u));
// 9u is the expected size, since one account object is a trustline
BEAST_EXPECT(checkArraySize(jro_l, 9u));
}
void
@@ -173,6 +174,7 @@ public:
// last item...with previous marker passed
jvParams[jss::marker] = jrr_l_2[jss::marker];
jvParams[jss::limit] = 10u;
auto const jrr_l_3 = env.rpc(
"json",
"account_offers",
@@ -203,9 +205,17 @@ public:
"account_offers",
jvParams.toStyledString())[jss::result];
auto const& jro = jrr[jss::offers];
BEAST_EXPECT(checkArraySize(jro, asAdmin ? 0u : 3u));
BEAST_EXPECT(
asAdmin ? checkMarker(jrr) : (!jrr.isMember(jss::marker)));
if (asAdmin)
{
// limit == 0 is invalid
BEAST_EXPECT(jrr.isMember(jss::error_message));
}
else
{
// Call should enforce min limit of 10
BEAST_EXPECT(checkArraySize(jro, 3u));
BEAST_EXPECT(!jrr.isMember(jss::marker));
}
}
}