[PR #2486] [MERGED] fix(pwntools): let callers widen the artifact input bound #5265

Closed
opened 2026-10-31 01:05:16 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2486
Author: @NIU-123370
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/pwntools-input-bound-override


📝 Commits (3)

  • 848082f fix(pwntools): let callers widen the artifact input bound
  • 5bb5de7 fix(pwntools): name the bound variable in its own refusal
  • 1729e87 chore: merge upstream CI baseline fixes

📊 Changes

7 files changed (+252 additions, -7 deletions)

View changed files

📝 docs/binary-diagnostics.md (+4 -2)
📝 docs/recorded-crashes.md (+7 -3)
📝 src/native/pwntools/PwntoolsDecoder.ts (+14 -1)
📝 src/native/pwntools/PwntoolsFailures.ts (+2 -1)
➕ src/native/pwntools/PwntoolsInputLimit.test.ts (+96 -0)
➕ src/native/pwntools/PwntoolsInputLimit.ts (+40 -0)
➕ tests/boundary/binaryDiagnostics/pwntoolsInputBoundAdmission.test.ts (+89 -0)

📄 Description

Summary

The pwntools adapters rejected every artifact above 32 MiB before any analysis ran, and no configuration surface could raise that bound, so ordinary recorded core dumps (the report's own 165 MiB example) were unanalyzable.

This adds REA_PWNTOOLS_MAX_INPUT_BYTES in front of the shipped default, which is the first option in #2446 — the default stays at 32 MiB so the existing protection is not weakened for anyone who does not opt in.

Fixes #2446

What changed

  • src/native/pwntools/PwntoolsInputLimit.ts (new) — one owner for the bound. undefined keeps PWNTOOLS_LIMITS.inputBytes; anything else must be a decimal integer from 1 to 1073741824. The ceiling is tied to the child's existing 3 GiB address-space reservation: the artifact is read into one in-memory buffer and staged again as a private snapshot, so one third of that reservation is the widest bound that still leaves the decoder room.
  • src/native/pwntools/PwntoolsDecoder.ts — resolves the bound from the adapter's own environment, after the existing REA_PWNTOOLS_PYTHON validation and before the artifact is read, then passes it to readStableArtifact. An unusable value is refused as provider_unavailable with the constraint text instead of silently falling back to the default. The other budgets (outputBytes, timeoutMs, diagnosticBytes) are untouched.
  • docs/recorded-crashes.md, docs/binary-diagnostics.md — the two places that stated the 32 MiB input bound now say it is raisable and what stays fixed.

What this deliberately does not do

Raising the input bound alone does not raise the reply, diagnostic or deadline budgets, so a core that needs more than 64 MiB of reply or 30 s of decoder time now fails at the decoder stage rather than at admission. That is the interaction #2446 describes. I kept the reply/deadline knobs out of this change because widening them is a different decision (it changes what the owned bridge may consume); if you want them configurable too, the same resolver shape extends to them directly.

Verification

src/native/pwntools/PwntoolsInputLimit.test.ts (18 cases, runs on any host) pins: the shipped default and its 32 MiB value; accepted values 1, 64 MiB, the report's 172879872, and the ceiling; and rejection of "", 0, -1, 33554432.0, 1e8, a leading space, 67_108_864, 0x2000000, one above the ceiling, and an integer beyond Number.MAX_SAFE_INTEGER.

tests/boundary/binaryDiagnostics/pwntoolsInputBoundAdmission.test.ts (3 cases) drives PwntoolsLayoutProvider with a 33,554,433-byte sparse regular file and asserts the projected caller-visible failure:

Case Projected result
no override invalid_request, detail contains Selected artifact exceeds 33554432 bytes
REA_PWNTOOLS_MAX_INPUT_BYTES=33554433 unreadable_output with the owned process's no reply captured — admission passed and the bridge really ran
REA_PWNTOOLS_MAX_INPUT_BYTES=0 provider_unavailable, detail contains the constraint, and no artifact-size text — refused before reading

Read this before reviewing those three cases: they are gated with it.runIf(!unsupportedHost) because the adapter itself refuses non-Linux/x64 hosts before reaching admission. I am on macOS, so to prove the wiring and the mutants I temporarily (a) commented out the adapter's host check and (b) flipped the test's unsupportedHost to false, ran the cases, then reverted both — the committed files contain the original gate and no bypass. On Linux/x64 they run unchanged; if they fail there I will fix them in this PR.

Mutation arms, each run, each killed by exactly one case:

Arm Killed by
read bound ignored (inputLimit.bytes → default) the widened-override admission case
override validation deleted the unusable-override admission case
default widened to 64 MiB the shipped-default case
regex dropped so any string parses 7 rejection cases
ceiling removed from the schema 2 rejection cases
empty string treated as unset the "" rejection case

Static gates on the rebased commit: tsc --noEmit clean; oxfmt --check clean; npx knip clean; oxlint 0 errors with 3 warnings that are byte-identical to the same file on untouched origin/main; vitest --project adapters 171 files / 1981 tests passed, 4 skipped.

Known collision surface

src/native/pwntools/PwntoolsDecoder.ts is also touched by #2281, #2282, #2284 and #2436. My edit there is one import, one validation block inserted before phase = "artifact-read", and one argument change, all inside the same guard sequence, so it should rebase cleanly; I will rebase again if it lands first.

docs/binary-diagnostics.md is touched by #2331 — a two-sentence prose edit.

Base: 2c7240cf. Head: 1 commit, 6 files.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2486 **Author:** [@NIU-123370](https://github.com/NIU-123370) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/pwntools-input-bound-override` --- ### 📝 Commits (3) - [`848082f`](https://github.com/morluto/rea/commit/848082fb3ad10b638a9698bef1b6f8c4fb0f8d50) fix(pwntools): let callers widen the artifact input bound - [`5bb5de7`](https://github.com/morluto/rea/commit/5bb5de73d2d547e6ebb3c6e333f302f46342a81d) fix(pwntools): name the bound variable in its own refusal - [`1729e87`](https://github.com/morluto/rea/commit/1729e87d9f2d1e9db797864cdfe763a327e13049) chore: merge upstream CI baseline fixes ### 📊 Changes **7 files changed** (+252 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `docs/binary-diagnostics.md` (+4 -2) 📝 `docs/recorded-crashes.md` (+7 -3) 📝 `src/native/pwntools/PwntoolsDecoder.ts` (+14 -1) 📝 `src/native/pwntools/PwntoolsFailures.ts` (+2 -1) ➕ `src/native/pwntools/PwntoolsInputLimit.test.ts` (+96 -0) ➕ `src/native/pwntools/PwntoolsInputLimit.ts` (+40 -0) ➕ `tests/boundary/binaryDiagnostics/pwntoolsInputBoundAdmission.test.ts` (+89 -0) </details> ### 📄 Description ## Summary The pwntools adapters rejected every artifact above 32 MiB before any analysis ran, and no configuration surface could raise that bound, so ordinary recorded core dumps (the report's own 165 MiB example) were unanalyzable. This adds `REA_PWNTOOLS_MAX_INPUT_BYTES` in front of the shipped default, which is the first option in #2446 — the default stays at 32 MiB so the existing protection is not weakened for anyone who does not opt in. Fixes #2446 ## What changed - `src/native/pwntools/PwntoolsInputLimit.ts` (new) — one owner for the bound. `undefined` keeps `PWNTOOLS_LIMITS.inputBytes`; anything else must be a decimal integer from 1 to 1073741824. The ceiling is tied to the child's existing 3 GiB address-space reservation: the artifact is read into one in-memory buffer and staged again as a private snapshot, so one third of that reservation is the widest bound that still leaves the decoder room. - `src/native/pwntools/PwntoolsDecoder.ts` — resolves the bound from the adapter's own environment, after the existing `REA_PWNTOOLS_PYTHON` validation and **before** the artifact is read, then passes it to `readStableArtifact`. An unusable value is refused as `provider_unavailable` with the constraint text instead of silently falling back to the default. The other budgets (`outputBytes`, `timeoutMs`, `diagnosticBytes`) are untouched. - `docs/recorded-crashes.md`, `docs/binary-diagnostics.md` — the two places that stated the 32 MiB input bound now say it is raisable and what stays fixed. ## What this deliberately does not do Raising the input bound alone does not raise the reply, diagnostic or deadline budgets, so a core that needs more than 64 MiB of reply or 30 s of decoder time now fails at the decoder stage rather than at admission. That is the interaction #2446 describes. I kept the reply/deadline knobs out of this change because widening them is a different decision (it changes what the owned bridge may consume); if you want them configurable too, the same resolver shape extends to them directly. ## Verification `src/native/pwntools/PwntoolsInputLimit.test.ts` (18 cases, runs on any host) pins: the shipped default and its 32 MiB value; accepted values `1`, 64 MiB, the report's 172879872, and the ceiling; and rejection of `""`, `0`, `-1`, `33554432.0`, `1e8`, a leading space, `67_108_864`, `0x2000000`, one above the ceiling, and an integer beyond `Number.MAX_SAFE_INTEGER`. `tests/boundary/binaryDiagnostics/pwntoolsInputBoundAdmission.test.ts` (3 cases) drives `PwntoolsLayoutProvider` with a 33,554,433-byte sparse regular file and asserts the projected caller-visible failure: | Case | Projected result | | --- | --- | | no override | `invalid_request`, detail contains `Selected artifact exceeds 33554432 bytes` | | `REA_PWNTOOLS_MAX_INPUT_BYTES=33554433` | `unreadable_output` with the owned process's `no reply` captured — admission passed and the bridge really ran | | `REA_PWNTOOLS_MAX_INPUT_BYTES=0` | `provider_unavailable`, detail contains the constraint, and no artifact-size text — refused before reading | **Read this before reviewing those three cases:** they are gated with `it.runIf(!unsupportedHost)` because the adapter itself refuses non-Linux/x64 hosts before reaching admission. I am on macOS, so to prove the wiring and the mutants I temporarily (a) commented out the adapter's host check and (b) flipped the test's `unsupportedHost` to `false`, ran the cases, then reverted both — the committed files contain the original gate and no bypass. On Linux/x64 they run unchanged; if they fail there I will fix them in this PR. Mutation arms, each run, each killed by exactly one case: | Arm | Killed by | | --- | --- | | read bound ignored (`inputLimit.bytes` → default) | the widened-override admission case | | override validation deleted | the unusable-override admission case | | default widened to 64 MiB | the shipped-default case | | regex dropped so any string parses | 7 rejection cases | | ceiling removed from the schema | 2 rejection cases | | empty string treated as unset | the `""` rejection case | Static gates on the rebased commit: `tsc --noEmit` clean; `oxfmt --check` clean; `npx knip` clean; `oxlint` 0 errors with 3 warnings that are byte-identical to the same file on untouched `origin/main`; `vitest --project adapters` 171 files / 1981 tests passed, 4 skipped. ## Known collision surface `src/native/pwntools/PwntoolsDecoder.ts` is also touched by #2281, #2282, #2284 and #2436. My edit there is one import, one validation block inserted before `phase = "artifact-read"`, and one argument change, all inside the same guard sequence, so it should rebase cleanly; I will rebase again if it lands first. `docs/binary-diagnostics.md` is touched by #2331 — a two-sentence prose edit. Base: `2c7240cf`. Head: 1 commit, 6 files. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:05:16 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5265
No description provided.