wallet: avoid int overflow in listtransactions #36339

pull kriss39 wants to merge 1 commits into bitcoin:master from kriss39:fix-listtransactions-count-overflow changing 2 files +21 −20

kriss39 commented at 9:38 AM on September 26, 2026: contributor
listtransactions adds count and skip as int. Each one is checked to be non-negative, but their sum can still go past INT_MAX:
```
bitcoin-cli -regtest listtransactions "*" 2147483647 1
```
On master this crashes the node. The overflowed sum is negative, so the loop stops after the first entry. The clamp after the loop overflows the same way and doesn't fix nCount, so push_backV gets an iterator range about 2^31 elements before rend(). UBSan reports signed integer overflow: 1 + 2147483647 at transactions.cpp:584, and ASan then reports a BUS error in UniValue::push_backV.

This does both additions in int64_t. The functional test calls listtransactions with count + skip above INT_MAX and checks the results match a normal-sized request. It times out on master (the node crashes) and passes with the fix. I also ran it on an ASan/UBSan build.

listrawtransactions counts differently and isn't affected.
DrahtBot added the label Wallet on Sep 26, 2026
DrahtBot commented at 9:38 AM on September 26, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36339.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #34872 (wallet: fix mixed-input transaction accounting in history RPCs by w0xlt)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.



LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
- ListTransactions(*pwallet, *pwtx, 0, true, ret, filter_label) in src/wallet/rpc/transactions.cpp
<sup>2026-09-28 08:27:42</sup>
kriss39 force-pushed on Sep 26, 2026
DrahtBot added the label CI failed on Sep 26, 2026
DrahtBot removed the label CI failed on Sep 26, 2026
in src/wallet/rpc/transactions.cpp:572 in 6aca05e7b9
maflcko commented at 8:05 AM on September 28, 2026:

This is unrelated, but pretty much every line here is problematic:

The default values are hard-coded a second time manually

The values are parsed as a singed int, but negative values are rejected manually

I'd say all of this can be avoided by using self.Arg<uint32_t>("count"). etc (or u64)

Possibly there could be an early check for AdditionOverflow<u64>(count,from) and early exit, to avoid having to manually cast later on?
kriss39 commented at 8:27 AM on September 28, 2026:

Thanks, done. count and skip now go through self.Arg<uint64_t>, so the duplicated defaults and the manual negative checks are gone.

For the sum I used SaturatingAdd rather than an early exit: something like count=UINT64_MAX, skip=1 is a valid "everything but the newest" request, so returning early would give the wrong answer there. With the saturated limit the rest is plain unsigned std::min, no casts.

Negative values now fail with "JSON integer out of range" (-1) instead of "Negative count"/"Negative from" (-8), same as other Arg<uint*> params. Test updated for that, plus a UINT64_MAX case. I left listrawtransactions alone to keep this small.

wallet: avoid int overflow in listtransactions

count and skip were parsed as int and checked to be non-negative, but
count + skip could still overflow, crashing the node for e.g.
`listtransactions "*" 2147483647 1`.

Parse both as uint64_t via self.Arg, which also takes care of the
defaults and rejects negative values, and saturate the sum so a huge
request just means "everything".

782e0ec4ff

kriss39 force-pushed on Sep 28, 2026

Contributors

Labels

wallet: avoid int overflow in listtransactions #36339

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)