From 1cbaec5d1f9183a0ac338a9f451b4b9b1fb5a5d2 Mon Sep 17 00:00:00 2001 From: Yousef Date: Tue, 28 Jul 2026 16:58:42 -0400 Subject: [PATCH 1/2] fix: Validate the tx_history start parameter doTxHistory read the start parameter with an unguarded asUInt(), which throws for negative, out-of-range, non-numeric and non-scalar values. The throw was swallowed by callMethod's catch-all, so a malformed start produced a generic internal error rather than invalidParams, and the request was recorded as a server fault in the perf log. Guard the conversion with the same isUInt()/isInt() check that readLimitField already applies to the limit field, and return invalidParams when it fails. This makes the handler stricter: a start given as a numeric string, null, or a bool was previously coerced and is now rejected. tx_history is registered for API version 1 only and is retired in version 2, so the affected surface is small. Fixes #6769 --- API-CHANGELOG.md | 1 + src/test/rpc/TransactionHistory_test.cpp | 32 +++++++++++++++++++ .../rpc/handlers/transaction/TxHistory.cpp | 9 +++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index a04f2653285..44a9efa26cf 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -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. [#NNNN](https://github.com/XRPLF/rippled/pull/NNNN) ## XRP Ledger server version 3.1.0 diff --git a/src/test/rpc/TransactionHistory_test.cpp b/src/test/rpc/TransactionHistory_test.cpp index 6227a1b75bb..e581cfe8892 100644 --- a/src/test/rpc/TransactionHistory_test.cpp +++ b/src/test/rpc/TransactionHistory_test.cpp @@ -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 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 diff --git a/src/xrpld/rpc/handlers/transaction/TxHistory.cpp b/src/xrpld/rpc/handlers/transaction/TxHistory.cpp index a45046773cc..560cb3aa157 100644 --- a/src/xrpld/rpc/handlers/transaction/TxHistory.cpp +++ b/src/xrpld/rpc/handlers/transaction/TxHistory.cpp @@ -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); From c4a3a11adf6cf0a126970093212c84d1ad90243c Mon Sep 17 00:00:00 2001 From: Yousef Date: Tue, 28 Jul 2026 17:27:50 -0400 Subject: [PATCH 2/2] docs: Link the tx_history changelog entry to its PR The entry was added with a placeholder because the pull request did not exist when the fix was committed. Point it at #7891. --- API-CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 44a9efa26cf..e5df7b4f49e 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -53,7 +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. [#NNNN](https://github.com/XRPLF/rippled/pull/NNNN) +- `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