[PR #3623] [CLOSED] fix(contracts): reject NUL bytes in rizin and cutter command inputs #5857

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3623
Author: @qorvr
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/contracts-command-nul-rejection


📝 Commits (1)

  • 30cbc3b fix(contracts): reject NUL bytes in rizin and cutter command inputs

📊 Changes

11 files changed (+372 additions, -24 deletions)

View changed files

📝 src/cli/cutterCommands.ts (+4 -5)
📝 src/cli/reverseEngineeringCommands.ts (+4 -5)
📝 src/contracts/cutterToolContracts.ts (+2 -7)
📝 src/contracts/rizinDebugToolContracts.ts (+2 -7)
➕ src/contracts/singleCommandLineSchema.test.ts (+178 -0)
➕ src/contracts/singleCommandLineSchema.ts (+14 -0)
📝 tests/boundary/cli/cutterCommands.test.ts (+100 -0)
📝 tests/boundary/cli/reverseEngineeringCommands.test.ts (+21 -0)
📝 tests/boundary/mcp/cutterMcp.test.ts (+12 -0)
📝 tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts (+9 -0)
📝 tests/boundary/providers/CutterBridgeService.test.ts (+26 -0)

📄 Description

Summary

Aligns cutter_command and rizin_debug_command single-line command validation with execute_rizin_command by rejecting embedded NUL (\u0000) bytes alongside \r and \n.

Problem and expected behavior

executeRizinCommandInputSchema in src/domain/reverseEngineering.ts already enforces !/[\r\n\u0000]/u.test(value) to prevent prematurely terminating command strings when passed to Rizin subprocesses or bridges. However, cutterCommandInputSchema (src/contracts/cutterToolContracts.ts), cutterCommand CLI (src/cli/cutterCommands.ts), rizin_debug_command (src/contracts/rizinDebugToolContracts.ts), and debug-with-rizin CLI (src/cli/reverseEngineeringCommands.ts) only checked !/[\r\n]/u.test(value).

When a caller supplies a single command line containing an embedded NUL byte, it could silently truncate commands at the NUL byte upon passing to the C/Python Rizin backend or corrupt the internal \n!echo ${marker}\n stream synchronization framing.

With this fix, single-line Cutter and Rizin commands consistently reject NUL bytes at the contract schema boundary before command dispatch or stream transmission.

Change and scope

  • In src/contracts/cutterToolContracts.ts: update cutterCommandInputSchema.command refinement to !/[\r\n\u0000]/u.test(value).
  • In src/contracts/rizinDebugToolContracts.ts: update rizin_debug_command input schema command refinement to !/[\r\n\u0000]/u.test(value).
  • In src/cli/cutterCommands.ts: update CLI cutter-command command argument refinement to !/[\r\n\u0000]/u.test(value).
  • In src/cli/reverseEngineeringCommands.ts: update CLI debug-with-rizin command argument refinement to !/[\r\n\u0000]/u.test(value).

Contract and boundary impact

  • Semantic owner and earliest changed stage: Contracts and CLI validation boundaries for Cutter and persistent Rizin debugger sessions.
  • Contribution scope fit and linked maintainer decision: Standard defensive input validation bug fix across existing contract boundaries; "not applicable".
  • CLI and MCP/tool-catalog contract: Unchanged public shape; rejects inputs with embedded NUL bytes with an input schema error.
  • Provider, bridge, target-format, or platform compatibility: Prevents premature C-string termination and command-stream desynchronization in Rizin and Cutter bridges.
  • Evidence, artifact, provenance, or reconstruction contract: None.
  • Target selection, authority, upstream behavior, and cleanup details: None.
  • Generated metadata, package, or installation impact: None.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Local validation:

  • oxfmt --check: clean across all 3,530 files.
  • tsc --noEmit: 0 errors.
  • Vitest suites passed 100%:
    • tests/boundary/mcp/cutterMcp.test.ts
    • tests/boundary/mcp/cutterCancellationMcp.test.ts
    • tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts
    • tests/boundary/providers/CutterBridgeClient.test.ts
    • tests/boundary/cli/reverseEngineeringCommands.test.ts
      (39/39 tests passed, 0 failed).

Readiness checklist

  • I followed the contribution scope.
  • I followed the code conventions.
  • I reviewed the relevant sections of DEVELOPMENT.md.
  • I confirmed the target branch is main.
  • I ensured this PR addresses one logical topic (maintainers may reject mixed topics).
  • I linked any tracking issue or explain why one does not exist.
  • I added tests covering regression or new behavior when practical.
  • I documented observable changes in the summary.
  • I completed the contract and boundary impact section above.
  • I updated generated catalogs or confirm that this change does not affect them.
  • I resolved any new linter, formatting, type, or dead-code issues.
  • I ran the appropriate test suites locally.
  • I selected readiness: Ready for review.

Merge checklist

  • All checks pass.
  • The commit history is linear and clean.
  • Commit message matches the PR title and follows conventional commit format.
  • Any non-trivial merge risk has been discussed with maintainers.

🔄 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/3623 **Author:** [@qorvr](https://github.com/qorvr) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/contracts-command-nul-rejection` --- ### 📝 Commits (1) - [`30cbc3b`](https://github.com/morluto/rea/commit/30cbc3b238084f3b25faffc34b6f368b585b2897) fix(contracts): reject NUL bytes in rizin and cutter command inputs ### 📊 Changes **11 files changed** (+372 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cutterCommands.ts` (+4 -5) 📝 `src/cli/reverseEngineeringCommands.ts` (+4 -5) 📝 `src/contracts/cutterToolContracts.ts` (+2 -7) 📝 `src/contracts/rizinDebugToolContracts.ts` (+2 -7) ➕ `src/contracts/singleCommandLineSchema.test.ts` (+178 -0) ➕ `src/contracts/singleCommandLineSchema.ts` (+14 -0) 📝 `tests/boundary/cli/cutterCommands.test.ts` (+100 -0) 📝 `tests/boundary/cli/reverseEngineeringCommands.test.ts` (+21 -0) 📝 `tests/boundary/mcp/cutterMcp.test.ts` (+12 -0) 📝 `tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts` (+9 -0) 📝 `tests/boundary/providers/CutterBridgeService.test.ts` (+26 -0) </details> ### 📄 Description ## Summary Aligns `cutter_command` and `rizin_debug_command` single-line command validation with `execute_rizin_command` by rejecting embedded NUL (`\u0000`) bytes alongside `\r` and `\n`. ## Problem and expected behavior `executeRizinCommandInputSchema` in `src/domain/reverseEngineering.ts` already enforces `!/[\r\n\u0000]/u.test(value)` to prevent prematurely terminating command strings when passed to Rizin subprocesses or bridges. However, `cutterCommandInputSchema` (`src/contracts/cutterToolContracts.ts`), `cutterCommand` CLI (`src/cli/cutterCommands.ts`), `rizin_debug_command` (`src/contracts/rizinDebugToolContracts.ts`), and `debug-with-rizin` CLI (`src/cli/reverseEngineeringCommands.ts`) only checked `!/[\r\n]/u.test(value)`. When a caller supplies a single command line containing an embedded NUL byte, it could silently truncate commands at the NUL byte upon passing to the C/Python Rizin backend or corrupt the internal `\n!echo ${marker}\n` stream synchronization framing. With this fix, single-line Cutter and Rizin commands consistently reject NUL bytes at the contract schema boundary before command dispatch or stream transmission. ## Change and scope - In `src/contracts/cutterToolContracts.ts`: update `cutterCommandInputSchema.command` refinement to `!/[\r\n\u0000]/u.test(value)`. - In `src/contracts/rizinDebugToolContracts.ts`: update `rizin_debug_command` input schema command refinement to `!/[\r\n\u0000]/u.test(value)`. - In `src/cli/cutterCommands.ts`: update CLI `cutter-command` command argument refinement to `!/[\r\n\u0000]/u.test(value)`. - In `src/cli/reverseEngineeringCommands.ts`: update CLI `debug-with-rizin` command argument refinement to `!/[\r\n\u0000]/u.test(value)`. ## Contract and boundary impact - Semantic owner and earliest changed stage: Contracts and CLI validation boundaries for Cutter and persistent Rizin debugger sessions. - Contribution scope fit and linked maintainer decision: Standard defensive input validation bug fix across existing contract boundaries; "not applicable". - CLI and MCP/tool-catalog contract: Unchanged public shape; rejects inputs with embedded NUL bytes with an input schema error. - Provider, bridge, target-format, or platform compatibility: Prevents premature C-string termination and command-stream desynchronization in Rizin and Cutter bridges. - Evidence, artifact, provenance, or reconstruction contract: None. - Target selection, authority, upstream behavior, and cleanup details: None. - Generated metadata, package, or installation impact: None. Expected effects: - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage Local validation: - `oxfmt --check`: clean across all 3,530 files. - `tsc --noEmit`: 0 errors. - Vitest suites passed 100%: - `tests/boundary/mcp/cutterMcp.test.ts` - `tests/boundary/mcp/cutterCancellationMcp.test.ts` - `tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts` - `tests/boundary/providers/CutterBridgeClient.test.ts` - `tests/boundary/cli/reverseEngineeringCommands.test.ts` (39/39 tests passed, 0 failed). ## Readiness checklist - [x] I followed the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). - [x] I followed the [code conventions](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#code-conventions). - [x] I reviewed the relevant sections of [DEVELOPMENT.md](https://github.com/morluto/rea/blob/main/DEVELOPMENT.md). - [x] I confirmed the target branch is `main`. - [x] I ensured this PR addresses one logical topic (maintainers may reject mixed topics). - [x] I linked any tracking issue or explain why one does not exist. - [x] I added tests covering regression or new behavior when practical. - [x] I documented observable changes in the summary. - [x] I completed the contract and boundary impact section above. - [x] I updated generated catalogs or confirm that this change does not affect them. - [x] I resolved any new linter, formatting, type, or dead-code issues. - [x] I ran the appropriate test suites locally. - [x] I selected readiness: `Ready for review`. ## Merge checklist - [x] All checks pass. - [x] The commit history is linear and clean. - [x] Commit message matches the PR title and follows conventional commit format. - [x] Any non-trivial merge risk has been discussed with maintainers. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:09:58 +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#5857
No description provided.