Skip to content

Commit

Permalink
APIv2(account_tx, noripple_check): return error on invalid input (XRP…
Browse files Browse the repository at this point in the history 
…LF#4620)

For the `account_tx` and `noripple_check` methods, perform input
validation for optional parameters such as "binary", "forward",
"strict", "transactions". Previously, when these parameters had invalid
values (e.g. not a bool), no error would be returned. Now, it returns an
`invalidParams` error.

* This updates the behavior to match Clio
  (https://github.com/XRPLF/clio).
* Since this is potentially a breaking change, it only applies to
  requests specifying api_version: 2.
* Fix XRPLF#4543.
  • Loading branch information
@PeterChen13579 @ckeshava
PeterChen13579 authored and ckeshava committed Sep 22, 2023
1 parent 8a400c0 commit 23d0991
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 135 deletions.
10 changes: 6 additions & 4 deletions src/ripple/rpc/handlers/AccountInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,12 @@ doAccountInfo(RPC::JsonContext& context)

result[jss::account_flags] = std::move(acctFlags);

// The document states that signer_lists is a bool, however
// assigning any string value works. Do not allow this.
// This check is for api Version 2 onwards only
if (!params[jss::signer_lists].isBool() && context.apiVersion > 1)
// The document[https://xrpl.org/account_info.html#account_info] states
// that signer_lists is a bool, however assigning any string value
// works. Do not allow this. This check is for api Version 2 onwards
// only
if (context.apiVersion > 1u && params.isMember(jss::signer_lists) &&
!params[jss::signer_lists].isBool())
{
RPC::inject_error(rpcINVALID_PARAMS, result);
return result;
Expand Down
19 changes: 17 additions & 2 deletions src/ripple/rpc/handlers/AccountTx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ parseLedgerArgs(RPC::Context& context, Json::Value const& params)
Json::Value response;
// if ledger_index_min or max is specified, then ledger_hash or ledger_index
// should not be specified. Error out if it is
if (context.apiVersion > 1)
if (context.apiVersion > 1u)
{
if ((params.isMember(jss::ledger_index_min) ||
params.isMember(jss::ledger_index_max)) &&
Expand Down Expand Up @@ -162,7 +162,7 @@ getLedgerRange(
// if ledger_index_min or ledger_index_max is out of
// valid ledger range, error out. exclude -1 as
// it is a valid input
if (context.apiVersion > 1)
if (context.apiVersion > 1u)
{
if ((ls.max > uValidatedMax && ls.max != -1) ||
(ls.min < uValidatedMin && ls.min != 0))
Expand Down Expand Up @@ -389,6 +389,21 @@ doAccountTxJson(RPC::JsonContext& context)
AccountTxArgs args;
Json::Value response;

// The document[https://xrpl.org/account_tx.html#account_tx] states that
// binary and forward params are both boolean values, however, assigning any
// string value works. Do not allow this. This check is for api Version 2
// onwards only
if (context.apiVersion > 1u && params.isMember(jss::binary) &&
!params[jss::binary].isBool())
{
return rpcError(rpcINVALID_PARAMS);
}
if (context.apiVersion > 1u && params.isMember(jss::forward) &&
!params[jss::forward].isBool())
{
return rpcError(rpcINVALID_PARAMS);
}

args.limit = params.isMember(jss::limit) ? params[jss::limit].asUInt() : 0;
args.binary = params.isMember(jss::binary) && params[jss::binary].asBool();
args.forward =
Expand Down
10 changes: 10 additions & 0 deletions src/ripple/rpc/handlers/NoRippleCheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ doNoRippleCheck(RPC::JsonContext& context)
if (params.isMember(jss::transactions))
transactions = params["transactions"].asBool();

// The document[https://xrpl.org/noripple_check.html#noripple_check] states
// that transactions params is a boolean value, however, assigning any
// string value works. Do not allow this. This check is for api Version 2
// onwards only
if (context.apiVersion > 1u && params.isMember(jss::transactions) &&
!params[jss::transactions].isBool())
{
return rpcError(rpcINVALID_PARAMS);
}

std::shared_ptr<ReadView const> ledger;
auto result = RPC::lookupLedger(ledger, context);
if (!ledger)
Expand Down
260 changes: 138 additions & 122 deletions src/test/rpc/AccountTx_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,176 +144,192 @@ class AccountTx_test : public beast::unit_test::suite
Json::Value jParms;
jParms[jss::api_version] = apiVersion;

if (apiVersion < 2)
{
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcINVALID_PARAMS));
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcINVALID_PARAMS));

jParms[jss::account] = "0xDEADBEEF";
jParms[jss::account] = "0xDEADBEEF";

BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcACT_MALFORMED));
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcACT_MALFORMED));

jParms[jss::account] = A1.human();
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(jParms))));
jParms[jss::account] = A1.human();
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(jParms))));

// Ledger min/max index
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = 0;
p[jss::ledger_index_max] = 100;
// Ledger min/max index
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = 0;
p[jss::ledger_index_max] = 100;
if (apiVersion < 2u)
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));

p[jss::ledger_index_min] = 1;
p[jss::ledger_index_max] = 2;
p[jss::ledger_index_min] = 1;
p[jss::ledger_index_max] = 2;
if (apiVersion < 2u)
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1
? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
rpcLGR_IDX_MALFORMED));

// Ledger index min only
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(apiVersion == 1 ? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
// Ledger index min only
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = 1;
p[jss::ledger_index_min] = 1;
if (apiVersion < 2u)
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = env.current()->info().seq;
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1
? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
rpcLGR_IDX_MALFORMED));

// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(apiVersion == 1 ? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}

p[jss::ledger_index_max] = env.current()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_max] = 3;
p[jss::ledger_index_max] = env.current()->info().seq;
if (apiVersion < 2u)
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));

p[jss::ledger_index_max] = env.closed()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = 3;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_max] = env.closed()->info().seq - 1;
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
}
p[jss::ledger_index_max] = env.closed()->info().seq;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

// Ledger Sequence
{
Json::Value p{jParms};
p[jss::ledger_index_max] = env.closed()->info().seq - 1;
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
}

p[jss::ledger_index] = env.closed()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
// Ledger Sequence
{
Json::Value p{jParms};

p[jss::ledger_index] = env.closed()->info().seq - 1;
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index] = env.closed()->info().seq;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_VALIDATED));
p[jss::ledger_index] = env.closed()->info().seq - 1;
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index] = env.current()->info().seq + 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_FOUND));
}
p[jss::ledger_index] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_VALIDATED));

// Ledger Hash
{
Json::Value p{jParms};
p[jss::ledger_index] = env.current()->info().seq + 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)), rpcLGR_NOT_FOUND));
}

p[jss::ledger_hash] = to_string(env.closed()->info().hash);
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
// Ledger Hash
{
Json::Value p{jParms};

p[jss::ledger_hash] =
to_string(env.closed()->info().parentHash);
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
}
p[jss::ledger_hash] = to_string(env.closed()->info().hash);
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_hash] = to_string(env.closed()->info().parentHash);
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
}
else

// Ledger index max/min/index all specified
// ERRORS out with invalid Parenthesis
{
// Ledger index max/min/index all specified
// ERRORS out with invalid Parenthesis
{
jParms[jss::account] = "0xDEADBEEF";
jParms[jss::account] = A1.human();
Json::Value p{jParms};
jParms[jss::account] = "0xDEADBEEF";
jParms[jss::account] = A1.human();
Json::Value p{jParms};

p[jss::ledger_index_max] = -1;
p[jss::ledger_index_min] = -1;
p[jss::ledger_index] = -1;
p[jss::ledger_index_max] = -1;
p[jss::ledger_index_min] = -1;
p[jss::ledger_index] = -1;

if (apiVersion < 2u)
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcINVALID_PARAMS));
}

// Ledger index min/max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = 100;
p[jss::ledger_index_min] = 0;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));
}

p[jss::ledger_index_max] = -1;
p[jss::ledger_index_min] = -1;
// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = env.current()->info().seq;
if (apiVersion < 2u)
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));

p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcINVALID_LGR_RANGE));
rpcLGR_IDX_MALFORMED));
}
// test binary and forward for bool/non bool values
{
Json::Value p{jParms};
p[jss::binary] = "asdf";
if (apiVersion < 2u)
{
Json::Value result{env.rpc("json", "account_tx", to_string(p))};
BEAST_EXPECT(result[jss::result][jss::status] == "success");
}
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcINVALID_PARAMS));

// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = env.current()->info().seq;
p[jss::binary] = true;
Json::Value result{env.rpc("json", "account_tx", to_string(p))};
BEAST_EXPECT(result[jss::result][jss::status] == "success");

p[jss::forward] = "true";
if (apiVersion < 2u)
BEAST_EXPECT(result[jss::result][jss::status] == "success");
else
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));
}
rpcINVALID_PARAMS));

p[jss::forward] = false;
result = env.rpc("json", "account_tx", to_string(p));
BEAST_EXPECT(result[jss::result][jss::status] == "success");
}
}

Expand Down
Loading

0 comments on commit 23d0991

Please sign in to comment.