[UTXO-BUG] /wallet/history: unbounded SQL fetchall causes OOM DoS

[Ivan-LB]

Bug

/wallet/history accepts limit and offset query parameters but the three inner SQL queries in api_wallet_history() had no SQL LIMIT clause. All matching rows were loaded into Python memory before pagination was applied in Python.

The ledger, epoch_rewards, and pending_ledger subqueries each call .fetchall() with no LIMIT, loading the wallet's entire history into RAM. The Python slice at the end only slices the already-loaded list and never prevents the database from scanning unbounded rows.

Exploit scenario

An attacker sends:

GET /wallet/history?miner_id=&limit=1&offset=0

The node responds with one row, but internally loads every ledger entry, every reward epoch, and every pending transfer for that wallet into RAM. On a mature node with hundreds of thousands of reward epochs, this can exhaust server memory and crash the process. No authentication is required because the endpoint is public.

Fix

Each SQL query now receives LIMIT = offset + limit, so the server never reads more rows than the caller can consume. One-line addition per query, no pagination semantics change.

The same pattern is applied to the epoch_rewards and pending_ledger subqueries.

Test file

node/test_wallet_history_oom_poc.py

Section A verifies via source scan that each of the three subqueries now contains a SQL LIMIT keyword. Section B verifies via direct SQLite queries that each bounded query returns at most limit rows when the DB contains far more.

All 7 tests pass after the fix.

Wallet

RTC64aa3fc417e75224e1574acae906fea34d94d140

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Non-doc PRs have a BCOS-L1 or BCOS-L2 label
• Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
• New code files include an SPDX license header
• You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

[eliasx45]

Reviewed current head 5573b21f35abecdffcc85e9770e6003213fa26ae.

Verdict: request changes.

The PR adds SQL LIMIT ? to the three /wallet/history inner queries, but the current implementation still has a high-offset OOM path and the new tests fail on Windows.

Primary issue: _history_cap = offset + limit is not bounded. Since offset is currently only clamped to >= 0, an unauthenticated caller can request something like GET /wallet/history?miner_id=<wallet>&limit=1&offset=1000000, and each inner query is allowed to read up to 1,000,001 rows into Python memory before the final slice. That preserves the core unbounded-memory risk for large offsets. Either use true SQL LIMIT ? OFFSET ? per source, cap offset, or otherwise bound the maximum rows fetched independent of caller offset.

Validation/repro:

git diff --check origin/main...HEAD
# clean

git merge-tree --write-tree origin/main HEAD
# ac3d121fdfa6b96108701039f25de81c1e3df74b

python -m py_compile node/rustchain_v2_integrated_v2.2.1_rip200.py node/test_wallet_history_oom_poc.py
# passed

python -m pytest -q node/test_wallet_history_oom_poc.py --tb=short
# FAILED: 3 failed, 4 passed
# UnicodeDecodeError: 'charmap' codec can't decode byte 0x90 ... while open(source_path).read()

python tools/bcos_spdx_check.py --base-ref origin/main
# FAILED: node/test_wallet_history_oom_poc.py is missing an SPDX header

The pytest failures come from _extract_history_queries() reading the integrated module without encoding="utf-8"; on this Windows checkout the default cp1252 codec cannot decode the file. Please make the regression portable, add the required SPDX header, and close the high-offset fetch path before this is ready.

[Ivan-LB]

Thanks for the detailed review. Three changes in the new commit:

High-offset OOM path closed. The offset parameter is now clamped at 9800 (offset = max(0, min(..., 9_800))), so the maximum rows any inner query can return is 9800 + 200 = 10000 regardless of what the caller sends. An offset=1_000_000 request no longer causes 1,000,001-row fetches.

Test portability. _extract_history_queries() now opens the source file with encoding="utf-8" instead of relying on the platform default, which fixes the UnicodeDecodeError on Windows cp1252 checkouts. All 8 tests pass locally.

SPDX header. Added # SPDX-License-Identifier: MIT as the first line of test_wallet_history_oom_poc.py. The SPDX check no longer flags this file.

[eliasx45]

Re-reviewed current head 98f54395650a2ffdd73427e47a3b93c02e068e13 after the follow-up commit. Verdict: approve.

The prior high-offset OOM path is now bounded: limit is capped at 200, offset is capped at 9800, and the inner SQL queries use _history_cap = offset + limit, so each source query is bounded to at most 10000 rows regardless of caller-supplied offset. The Windows portability and SPDX blockers are also fixed.

Validation on this head:

git diff --check origin/main...HEAD
# clean

git merge-tree --write-tree origin/main HEAD
# 9d7599fac6a5c53a4d9f0e576842540f4a2f10ef

python -m py_compile node/rustchain_v2_integrated_v2.2.1_rip200.py node/test_wallet_history_oom_poc.py
# passed

python -m pytest -q node/test_wallet_history_oom_poc.py --tb=short
# 8 passed

python tools/bcos_spdx_check.py --base-ref origin/main
# BCOS SPDX check: OK

Scope note: this bounds the single-request /wallet/history memory path. It does not make wallet history private, but that is separate from the OOM fetchall fix reviewed here.

[Ivan-LB]

Thanks for re-reviewing the head and running the full validation suite. Good to know the _history_cap = offset + limit bound holds across all three subqueries and the SPDX and Windows checks are clean.

[Ivan-LB]

Updated the test file to address the feedback from the previous review.

Section B now imports the real Flask app via importlib and calls GET /wallet/history through app.test_client() directly. The stub _build_app() and the four standalone-SQL tests have been replaced with TestWalletHistoryFlaskRoute, a unittest.TestCase class with 7 tests that exercise the real route: ok field, default limit bound, explicit limit enforcement, large-offset capping (999999 → 9800 silently), missing miner_id returning 400, non-integer limit returning 400, non-integer offset returning 400.

Section A source-scan tests are unchanged. All 11 tests pass locally.

[eliasx45]

Re-reviewed current head 9277e4123a7d7f60a53b7b422ca6d5489a5472b7 after the test rewrite.

Verdict: approve.

The new Section B coverage is a material improvement over the stubbed route tests: it imports the real integrated Flask app and exercises GET /wallet/history through app.test_client(). The regression now covers the real route returning ok: true, default limit behavior, explicit limit enforcement, large-offset capping, missing miner_id, non-integer limit, and non-integer offset. The source-scan Section A still verifies the SQL LIMIT / _history_cap / offset-cap structure.

Validation on this Windows checkout:

git diff --check origin/main...HEAD
# clean

git merge-tree --write-tree origin/main HEAD
# 24781afc2e36ccc47ec7126f13266f3bf9b18a64

..\Rustchain\.venv\Scripts\python.exe -m py_compile node\rustchain_v2_integrated_v2.2.1_rip200.py node\test_wallet_history_oom_poc.py
# passed

PYTHONPATH=node ..\Rustchain\.venv\Scripts\python.exe -m pytest -q node\test_wallet_history_oom_poc.py --tb=short
# 11 passed

..\Rustchain\.venv\Scripts\python.exe tools\bcos_spdx_check.py --base-ref origin/main
# BCOS SPDX check: OK

The reviewed scope remains the single-request /wallet/history OOM bound; privacy/enumeration concerns are separate from this route-memory fix.

[Ivan-LB]

Thanks for re-reviewing after the test rewrite and validating on Windows. Glad the Section B coverage landed well — the integrated Flask client tests against the real route were the right call over the stubbed approach. Appreciate the thorough sign-off.

[jaxint]

LGTM! Great work on this PR. 🚀

[crystal-tensor]

LGTM! Code review approved by @cx95zz (QClaw automated review agent).

Reviewed for: correctness, security, test coverage, and code quality.

No issues found - APPROVED.

[Scottcjn]

Merged + paid 25 RTC (Medium #2867 — /wallet/history OOM). Reserved at RTC64aa3fc4… per your prior wallet declaration on #6510. 24h void window applies. Thanks @Ivan-LB.