rpc: bound memory for overlong Bech32 errors #36111

l0rinc commented at 4:49 AM on August 28, 2026: contributor

Problem: validateaddress reports likely error positions for invalid Bech32 inputs, including multiple useful positions for character and checksum errors. For an overlength input, LocateErrors() returns every position after the 90-character limit, which the RPC converts to a UniValue number before serializing the response. A near-limit authenticated request therefore creates about 33 million int values and 33 million UniValue objects.

Fix: Return position 90 for an overlength input, which identifies where the single length violation begins. Character and checksum errors continue to return multiple useful positions when they can be determined. The tests now include an oversized example and pin the bounded result.

Reproducer: Peak memory usage for a near-limit authenticated request:

<details> <summary>Linux reproducer</summary>

sed -i "/def test_validateaddress(self):/a\\
        self.nodes[0].validateaddress('bcrt1' + 'q' * (2**25 - 100))\\
        __import__('time').sleep(30)" test/functional/rpc_invalid_address_message.py
cmake -B build && cmake --build build -j2
build/test/functional/rpc_invalid_address_message.py >/dev/null 2>&1 &
sleep 20 && awk '/VmHWM/' /proc/$(pgrep bitcoind)/status

</details>

Before ████████████████████████ 5.69 GiB
After  █░░░░░░░░░░░░░░░░░░░░░░░  240 MiB

bech32: bound overlength error locations

`LocateErrors()` returns multiple useful positions for character and checksum errors, but an overlength string has one structural error.
Every character from the limit onward is outside the permitted address, so listing each position adds no diagnostic value.
`validateaddress` converts every returned position into a `UniValue` number before serializing the response.
An authenticated request below the HTTP body limit can therefore require several gigabytes of memory.

Return only the first position beyond the length limit, which identifies where the violation begins.
Character and checksum errors continue to report multiple useful positions when they can be determined, and the existing unit and functional tests cover both behaviors.

7fcaccd9d0

DrahtBot added the label RPC/REST/ZMQ on Aug 28, 2026

DrahtBot commented at 4:49 AM on August 28, 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/36111.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	maflcko, sedited, janb84

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36087 (util: Add and use AssertUnreachable by maflcko)
#27260 (Enhanced error messages for invalid network prefix during address parsing. by portlandhodl)

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.

maflcko commented at 6:23 AM on August 28, 2026: member

lgtm ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d

sedited approved

sedited commented at 8:30 AM on August 28, 2026: contributor

ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d

I'm curious why this was done in the first place. This was introduced in #16807 and refactored in #23577 's https://github.com/bitcoin/bitcoin/commit/2fa4fd196176160a5ad0a25da173ff93252b8103 , so it seems like some intent went into it and wasn't just coincidentally applying the same pattern.

maflcko commented at 9:22 AM on August 28, 2026: member

I presume it just wasn't considered that a user will pass more than 100 or 200 chars?

in src/bech32.cpp:407 in 7fcaccd9d0

 402 | @@ -404,8 +403,7 @@ std::pair<std::string, std::vector<int>> LocateErrors(const std::string& str, Ch
 403 |      std::vector<int> error_locations{};
 404 |  
 405 |      if (str.size() > limit) {
 406 | -        error_locations.resize(str.size() - limit);
 407 | -        std::iota(error_locations.begin(), error_locations.end(), static_cast<int>(limit));
 408 | +        error_locations.push_back(static_cast<int>(limit));
 409 |          return std::make_pair("Bech32 string too long", std::move(error_locations));

janb84 commented at 10:51 AM on August 28, 2026:

NIT, non blocking: the change removes the need of the vector of error_locations, just return the single limit position violation directly

 return std::make_pair("Bech32 string too long", std::vector<int>{static_cast<int>(limit)});

l0rinc commented at 3:02 PM on August 28, 2026:

Thanks, will take these if I need to retouch

janb84 commented at 10:56 AM on August 28, 2026: contributor

ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d

This PR fixes the over-length bech32 string balooning issue, this also changes the RPC error a bit (returning positions) but it's not enough to warrant a release note, IMHO (The error message stays the same)

main:

{
  "isvalid": false,
  "error": "Bech32 string too long",
  "error_locations": [90, 91, 92, ..., 107]
}

this pr:

{
  "isvalid": false,
  "error": "Bech32 string too long",
  "error_locations": [90]
}

</details>

Added one non blocking NIT, removing a unnecessary mutation of a vector.

in src/bech32.cpp:377 in 7fcaccd9d0

janb84 commented at 11:20 AM on August 28, 2026:

NIT: move the string length check up so the over-length string will get rejected early before scanning it with CheckCharacters() creating a big error vector that's thrown away.

if (str.size() > limit) return {};
std::vector<int> errors;
if (!CheckCharacters(str, errors)) return {};
size_t pos = str.rfind(SEPARATOR);

sedited merged this on Aug 29, 2026

sedited closed this on Aug 29, 2026