APIv2(account_tx, noripple_check): return error for invalid input (fix #4543)

src/ripple/rpc/handlers/AccountInfo.cpp

@@ -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;

src/ripple/rpc/handlers/AccountTx.cpp

@@ -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)) &&
@@ -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))
@@ -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 =

src/ripple/rpc/handlers/NoRippleCheck.cpp

@@ -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)

src/test/rpc/AccountTx_test.cpp

@@ -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");
  }
  }