Skip to content

#2144: wallet-api: reject deeply nested JSON instead of overflowing the stack - #2145

Closed
profinch wants to merge 1 commit into
BeamMW:masterfrom
profinch:fix/2144-wallet-api-json-depth
Closed

profinch wants to merge 1 commit into
BeamMW:masterfrom
profinch:fix/2144-wallet-api-json-depth

Conversation

@profinch

@profinch profinch commented Oct 6, 2026

Copy link
Copy Markdown

Fixes #2144.

The vendored nlohmann::json (3.1.2) parses recursively with no depth limit. A single unauthenticated HTTP request such as [[[[...]]]] (80 000 levels, about 160 KB) overflowed the stack and killed wallet-api.

Fix

  • ApiBase pre-scans the request before json::parse: one pass, no allocations. It tracks string and escape state so brackets inside strings don't count.
  • Nesting deeper than 128 levels is rejected with InvalidJsonRpc ("JSON nesting is too deep."). Real API requests nest only a few levels. 128 matches serde_json's default.
  • fromError re-parses the request to recover its id (apps API path), so it is guarded the same way.
  • The check sits in ApiBase, so it covers every transport: HTTP, TCP, websockets, apps API and wasm.

Tests

  • wallet_api_test: a 1 000 000-level array is rejected (segfaulted before the fix). Depth 129 is rejected and depth 128 still parses. Brackets inside strings and after escaped quotes are not counted.
  • Manual: wallet-api --use_http=1 with the repro from the issue. The unfixed binary dies at depth 80 000. The fixed binary returns -32600 JSON nesting is too deep. for depths 80 000 and 400 000, and keeps serving wallet_status afterwards.

…wing the stack

nlohmann::json 3.1.2 parses recursively with no depth limit, so a single
unauthenticated HTTP request like [[[[...]]]] crashed wallet-api. ApiBase now
pre-scans the request (O(n), no allocations) and rejects nesting deeper than
128 levels with InvalidJsonRpc before json::parse. fromError, which re-parses
the request to recover its id, is guarded the same way.
@profinch

profinch commented Oct 6, 2026

Copy link
Copy Markdown
Author

See also #2146, an alternative fix for #2144 that applies the same pre-scan (also limited to 128) to every place that parses JSON from outside, not only ApiBase. Checked against master, these also crash with one unauthenticated input and are not covered here:

  • an SBBS instant message from anyone who knows the address, crashing wallet-api on read_messages;
  • the beam-node stratum port: one line of 65 000 [ (the parser recurses on opening brackets, so the 64 KB line limit doesn't protect it);
  • replies from swap bridge servers (Electrum, bitcoind, Ethereum).

The TCP mode of wallet-api is covered here too, since it goes through ApiBase, although the issue calls it safe: 65 000 unclosed [ in one line crash master.

@profinch

profinch commented Oct 6, 2026 •

Copy link
Copy Markdown
Author

Note: this also fixes the default TCP mode, which turned out to be exposed as well (one 64 KB line of [ crashes it). Details and repro: #2144 (comment). Verified against the fixed binary: TCP mode returns -32600 JSON nesting is too deep. and stays up.

@profinch

profinch commented Oct 6, 2026

Copy link
Copy Markdown
Author

Closing in favour of #2146: it fixes the same bug with one shared helper and also covers the stratum server (#2147), swap bridges, the explorer and read_messages. Reviewed and tested there.

@profinch profinch closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wallet-api (HTTP mode): unauthenticated crash (stack overflow) on deeply-nested JSON

1 participant