Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions API-CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ This section contains changes targeting a future version.
- `submit`: The `fail_hard` field now returns an error if the value is not a boolean. [#6529](https://github.com/XRPLF/rippled/pull/6529)
- `subscribe`: The `taker` field in the `books` array now returns `actMalformed` instead of `badIssuer` if the value is not a valid account. [#6529](https://github.com/XRPLF/rippled/pull/6529)
- Fixed a bug in `Forwarded` HTTP header parsing where the extracted IP address could be incorrect when no comma or semicolon delimiter follows the address. This could cause the server to misidentify a client's IP address when operating behind a reverse proxy. [#6529](https://github.com/XRPLF/rippled/pull/6529)
- `tx_history`: The `start` field now returns `invalidParams` if the value is not a non-negative integer. Previously, values that could not be converted returned an `internal` error, and strings, booleans, and `null` were silently coerced. [#7891](https://github.com/XRPLF/rippled/pull/7891)

## XRP Ledger server version 3.1.0

Expand Down
32 changes: 32 additions & 0 deletions src/test/rpc/TransactionHistory_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,38 @@ class TransactionHistory_test : public beast::unit_test::Suite
BEAST_EXPECT(result[jss::error] == "noPermission");
BEAST_EXPECT(result[jss::status] == "error");
}

{
// a malformed start must report invalidParams, not internal
json::Value objStart{json::ValueType::Object};
objStart["nope"] = 0;
json::Value arrStart{json::ValueType::Array};
arrStart.append(0);

// Note that a whole number beyond the range of a uint, e.g.
// 4294967296, is rejected by the json parser itself, so it never
// reaches the handler and is not covered here.
boost::container::static_vector<json::Value, 9> const badStarts{
json::Value{-1}, // negative
json::Value{"abc"}, // not a number
json::Value{"5"}, // numeric, but a string
json::Value{1.5}, // not a whole number
json::Value{1e30}, // beyond the range of a uint
json::Value{true}, // bool
json::Value{}, // null
objStart,
arrStart,
};

for (auto const& badStart : badStarts)
{
json::Value params{json::ValueType::Object};
params[jss::start] = badStart;
auto const result = env.client().invoke("tx_history", params)[jss::result];
BEAST_EXPECT(result[jss::error] == "invalidParams");
BEAST_EXPECT(result[jss::status] == "error");
}
}
}

void
Expand Down
9 changes: 8 additions & 1 deletion src/xrpld/rpc/handlers/transaction/TxHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ doTxHistory(RPC::JsonContext& context)
if (!context.params.isMember(jss::start))
return rpcError(RpcInvalidParams);

unsigned int const startIndex = context.params[jss::start].asUInt();
// Guard the conversion: json::Value::asUInt() throws for negative,
// out-of-range, non-numeric or non-scalar values, which would surface as a
// generic internal error instead of invalid parameters.
auto const& jvStart = context.params[jss::start];
if (!jvStart.isUInt() && (!jvStart.isInt() || jvStart.asInt() < 0))
return rpcError(RpcInvalidParams);

unsigned int const startIndex = jvStart.asUInt();

if ((startIndex > 10000) && (!isUnlimited(context.role)))
return rpcError(RpcNoPermission);
Expand Down