[GH-ISSUE #7430] [Bug] Android CLI accepts an empty overload index instead of rejecting it #4435

New issue

Closed

opened 2026-10-31 00:49:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:49:47 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 27, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7430

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

CLI

Observed behavior

node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select --overload-index= --json accepts the empty index and reaches artifact admission (target_unavailable). Replacing the empty value with x produces VALIDATION_ERROR for overloadIndex before artifact admission. trace-android-references with --method-name=select accepts the same empty option. Source-derived consequence: the CLI converts the empty token into an explicit index 0, bypassing the documented requirement to choose an overload when the method name is ambiguous.

Reproduction

Use Node.js 24.22.1 and the revision below. A fresh checkout needs npm ci && npm run build:cached; verification reused the locked dependencies after confirming the lockfile was unchanged, then ran npm run build:cached. Run from the checkout root, with ./missing.apk deliberately absent:

WORK="$(mktemp -d)"
mkdir -p "$WORK"/{home,config,data,cache,state,temp}
export HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config" XDG_DATA_HOME="$WORK/data"
export XDG_CACHE_HOME="$WORK/cache" XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/temp"
for option in --overload-index= --overload-index=x --overload-index=0 --overload-index=0x1 omitted; do
  if [ "$option" = omitted ]; then
    node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select --json > "$WORK/result.json"
  else
    node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select "$option" --json > "$WORK/result.json"
  fi
  result_status=$?
  node -e 'const d=require(process.argv[1]);console.log(JSON.stringify({option:process.argv[2],exit:Number(process.argv[3]),code:d.code,fields:(d.fieldErrors||[]).map(f=>f.path)}))' "$WORK/result.json" "$option" "$result_status"
done
rm -rf "$WORK"

Each listed case ran twice with the same result. The corresponding reference-tracing cases were run once. No APK or external analysis provider is needed to observe the validation failure.

Expected behavior

Reject an explicitly empty overload index with a field-specific validation error before artifact admission. Preserve omission and explicit decimal zero. src/cli/androidCommands.ts:73-86 describes explicit zero-based overload selection, and docs/android-analysis.md:191-195 promises to reject ambiguous method requests rather than silently select the first match. src/domain/android/androidAnalysis.ts:24-50 declares optional nonnegative integer selectors. The existing decimal-token helper at src/cli/options.ts:16-21 preserves the distinction between integer text and Number-coercible text.

Impact and scope

P2: a script passing an empty index variable can silently request overload 0 rather than leave the method unselected or report invalid input. The public CLI observations establish incorrect admission, not completed decompilation. Real APK/JADX execution and downstream analyst consequences were not tested. Omitting the flag or supplying a validated decimal index avoids the malformed input; no data loss or security boundary crossing was observed.

REA version

Exact node scripts/rea.mjs --version output: 6.8.0. Source commit: f2277454b8c5356abbe311ad6dbb1dd3a3e334a5.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; arm64. Built runtime and locked dependencies; no real Android analysis engine or device.

Target and artifact context

A deliberately absent local APK path isolates argument validation from artifact admission. No private APK, target execution, or real provider was used.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Observed output from the reproduction above; all commands had empty stderr:

{"option":"--overload-index=","exit":1,"code":"target_unavailable","fields":[]}
{"option":"--overload-index=x","exit":1,"code":"VALIDATION_ERROR","fields":["overloadIndex"]}
{"option":"--overload-index=0","exit":1,"code":"target_unavailable","fields":[]}
{"option":"--overload-index=0x1","exit":1,"code":"target_unavailable","fields":[]}
{"option":"omitted","exit":1,"code":"target_unavailable","fields":[]}

Separately observed over production stdio MCP: inspect_android_method with {"path":"./missing.apk","class_name":"com.example.Main","method_name":"select","overload_index":""} returned invalid_request with overload_index: Invalid input: expected number, received string. Numeric 0 reached target_unavailable. Its advertised property was {"type":"integer","minimum":0,"maximum":9007199254740991}. These observations support the empty-value distinction; JSON numeric spelling and CLI token syntax are different representations.

Typed outcome or recovery expected

A nonzero VALIDATION_ERROR naming overloadIndex, before opening the artifact. Proposed restoration checks: reject empty and hex tokens for both commands, while preserving omitted-index ambiguity rejection and explicit decimal selection. No repaired build was tested.

Analysis or suspected cause

Source-derived: src/cli/androidCommands.ts:79-96 and :108-124 expose numeric schemas and forward every defined value. The locked Incur parser applies Number(value) before numeric validation (node_modules/incur/src/Parser.ts:323-330), erasing empty text into zero and hex text into an integer. src/application/android/AndroidAnalysisService.ts:35-57 accepts that numeric zero. src/android/JadxAnalysis.ts:235-255 and :318-349 reject ambiguity only when the index is undefined and otherwise select the indexed candidate. The result reports the selected overload's identity; the lost caller intent occurs at CLI parsing.

Additional context

Related open #7376 concerns byte-window selectors, and open #7093 concerns listener ports. Closed PR #7156 changes only the listener's port schema. A common integer-parser cleanup could address several options, but those reports do not cover Android overload selection. No exact duplicate was found. The first affected release is unknown.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: The supported CLI accepts empty selector text and proceeds past validation; source tracing establishes conversion to explicit overload zero and bypass of the ambiguity guard. Real-provider selection and any downstream harm were not observed.
Workaround status: available
Workaround basis: Omission and decimal zero both passed the same CLI boundary probes, and the source preserves omission for ambiguity checks. Callers can omit an unset flag or validate a decimal index before supplying it.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7430 ## Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ## Area CLI ## Observed behavior `node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select --overload-index= --json` accepts the empty index and reaches artifact admission (`target_unavailable`). Replacing the empty value with `x` produces `VALIDATION_ERROR` for `overloadIndex` before artifact admission. `trace-android-references` with `--method-name=select` accepts the same empty option. Source-derived consequence: the CLI converts the empty token into an explicit index 0, bypassing the documented requirement to choose an overload when the method name is ambiguous. ## Reproduction Use Node.js 24.22.1 and the revision below. A fresh checkout needs `npm ci && npm run build:cached`; verification reused the locked dependencies after confirming the lockfile was unchanged, then ran `npm run build:cached`. Run from the checkout root, with `./missing.apk` deliberately absent: ```sh WORK="$(mktemp -d)" mkdir -p "$WORK"/{home,config,data,cache,state,temp} export HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config" XDG_DATA_HOME="$WORK/data" export XDG_CACHE_HOME="$WORK/cache" XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/temp" for option in --overload-index= --overload-index=x --overload-index=0 --overload-index=0x1 omitted; do if [ "$option" = omitted ]; then node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select --json > "$WORK/result.json" else node scripts/rea.mjs inspect-android-method ./missing.apk com.example.Main select "$option" --json > "$WORK/result.json" fi result_status=$? node -e 'const d=require(process.argv[1]);console.log(JSON.stringify({option:process.argv[2],exit:Number(process.argv[3]),code:d.code,fields:(d.fieldErrors||[]).map(f=>f.path)}))' "$WORK/result.json" "$option" "$result_status" done rm -rf "$WORK" ``` Each listed case ran twice with the same result. The corresponding reference-tracing cases were run once. No APK or external analysis provider is needed to observe the validation failure. ## Expected behavior Reject an explicitly empty overload index with a field-specific validation error before artifact admission. Preserve omission and explicit decimal zero. `src/cli/androidCommands.ts:73-86` describes explicit zero-based overload selection, and `docs/android-analysis.md:191-195` promises to reject ambiguous method requests rather than silently select the first match. `src/domain/android/androidAnalysis.ts:24-50` declares optional nonnegative integer selectors. The existing decimal-token helper at `src/cli/options.ts:16-21` preserves the distinction between integer text and Number-coercible text. ## Impact and scope P2: a script passing an empty index variable can silently request overload 0 rather than leave the method unselected or report invalid input. The public CLI observations establish incorrect admission, not completed decompilation. Real APK/JADX execution and downstream analyst consequences were not tested. Omitting the flag or supplying a validated decimal index avoids the malformed input; no data loss or security boundary crossing was observed. ## REA version Exact `node scripts/rea.mjs --version` output: `6.8.0`. Source commit: `f2277454b8c5356abbe311ad6dbb1dd3a3e334a5`. ## Runtime and platform Node.js v24.22.1; macOS 27.0.1; arm64. Built runtime and locked dependencies; no real Android analysis engine or device. ## Target and artifact context A deliberately absent local APK path isolates argument validation from artifact admission. No private APK, target execution, or real provider was used. ## Interface CLI ## Earliest failure stage Request parsing or validation ## Relevant output or evidence Observed output from the reproduction above; all commands had empty stderr: ```json {"option":"--overload-index=","exit":1,"code":"target_unavailable","fields":[]} {"option":"--overload-index=x","exit":1,"code":"VALIDATION_ERROR","fields":["overloadIndex"]} {"option":"--overload-index=0","exit":1,"code":"target_unavailable","fields":[]} {"option":"--overload-index=0x1","exit":1,"code":"target_unavailable","fields":[]} {"option":"omitted","exit":1,"code":"target_unavailable","fields":[]} ``` Separately observed over production stdio MCP: `inspect_android_method` with `{"path":"./missing.apk","class_name":"com.example.Main","method_name":"select","overload_index":""}` returned `invalid_request` with `overload_index: Invalid input: expected number, received string`. Numeric `0` reached `target_unavailable`. Its advertised property was `{"type":"integer","minimum":0,"maximum":9007199254740991}`. These observations support the empty-value distinction; JSON numeric spelling and CLI token syntax are different representations. ## Typed outcome or recovery expected A nonzero `VALIDATION_ERROR` naming `overloadIndex`, before opening the artifact. Proposed restoration checks: reject empty and hex tokens for both commands, while preserving omitted-index ambiguity rejection and explicit decimal selection. No repaired build was tested. ## Analysis or suspected cause Source-derived: `src/cli/androidCommands.ts:79-96` and `:108-124` expose numeric schemas and forward every defined value. The locked Incur parser applies `Number(value)` before numeric validation (`node_modules/incur/src/Parser.ts:323-330`), erasing empty text into zero and hex text into an integer. `src/application/android/AndroidAnalysisService.ts:35-57` accepts that numeric zero. `src/android/JadxAnalysis.ts:235-255` and `:318-349` reject ambiguity only when the index is undefined and otherwise select the indexed candidate. The result reports the selected overload's identity; the lost caller intent occurs at CLI parsing. ## Additional context Related open [#7376](https://github.com/morluto/rea/issues/7376) concerns byte-window selectors, and open [#7093](https://github.com/morluto/rea/issues/7093) concerns listener ports. Closed [PR #7156](https://github.com/morluto/rea/pull/7156) changes only the listener's port schema. A common integer-parser cleanup could address several options, but those reports do not cover Android overload selection. No exact duplicate was found. The first affected release is unknown. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: The supported CLI accepts empty selector text and proceeds past validation; source tracing establishes conversion to explicit overload zero and bypass of the ambiguity guard. Real-provider selection and any downstream harm were not observed. - Workaround status: available - Workaround basis: Omission and decimal zero both passed the same CLI boundary probes, and the source preserves omission for ambiguity checks. Callers can omit an unset flag or validate a decimal index before supplying it.

bsakaguchi closed this issue

2026-10-31 00:49:47 +00:00

bsakaguchi referenced this issue

2026-10-31 01:12:58 +00:00

[PR #4435] [MERGED] fix(pwntools): preserve worker ownership across launcher exec #6241