Skip to content

Fix silent feature corruption: register-as-number parsing, x64 negative masks, duplicate string/stack-string features - #25

Open
mnaza wants to merge 2 commits into
marirs:masterfrom
mnaza:fix/extractor-feature-corruption
Open

Fix silent feature corruption: register-as-number parsing, x64 negative masks, duplicate string/stack-string features#25
mnaza wants to merge 2 commits into
marirs:masterfrom
mnaza:fix/extractor-feature-corruption

Conversation

@mnaza

@mnaza mnaza commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Closes #24.

Four extraction bugs that silently emit wrong or duplicate features. 3 regression tests added (16 pass, was 13).

1. Registers ah/bh/ch/dh and hex-looking labels parsed as numbers

parse_operand_to_number case 2 stripped the h suffix and parsed the rest as hex: "ah""a"Some(10), so mov al, ah emitted Number(0xA). Case 5 (bare hex fallback) also accepted labels like beef/face. Both paths now require a leading digit — the Intel convention for hex literals (0ABh keeps working; ah, beef do not parse).

2. Negative immediates masked to 32 bits on x64

(s as u32) as i128mov rax, -1 emitted Number(0xFFFFFFFF) instead of Number(0xFFFFFFFFFFFFFFFF). New mask_to_bitness helper follows the function's bitness.

3. stack string pushed once per remaining instruction

The push sat inside the instruction loop with no break, so a block crossing the threshold got one duplicate characteristic per remaining instruction. Now emitted once per basic block.

4. Every ASCII string emitted twice

extract_file_strings calls extract_ascii_strings and extract_unicode_strings — but the latter also ran a plain-ASCII pass ([\x20-\x7E]{4,}, the same printable class). Each ASCII string landed twice in the string feature set: double matching work for every string rule and duplicate VAs in feature maps. The Unicode extractor is now UTF-16-only; ASCII coverage stays with extract_ascii_strings (whose class is slightly wider — includes \t).

Out of scope (noticed while testing, pre-existing)

Case 3 of parse_operand_to_number (-0x10 / +0x10 sign forms) is dead code: RE_NUMBER_HEX captures the number with the 0x prefix, and i128::from_str_radix rejects it — so signed hex immediates never parse. Left alone here to keep the PR focused; happy to file a follow-up if you want it fixed.

Clippy clean, cargo fmt applied, CHANGELOG entry under [Unreleased]. Based on master; independent of the other open PRs (#17 touches the same file but different regions; only the [Unreleased] CHANGELOG heading is shared).

@marirs

marirs commented Jul 30, 2026

Copy link
Copy Markdown
Owner

All 4 pull requests has conflicts

- parse_operand_to_number: require a leading digit for h-suffixed and
  bare hex literals — register names ah/bh/ch/dh parsed as 0xA-0xD and
  hex-looking labels (beef, face) parsed as numbers.
- mask negative immediates at the function's bitness (was always u32,
  truncating x64 values like mov rax, -1 to 0xFFFFFFFF).
- emit stack string characteristic once per basic block (was pushed
  per instruction past the threshold with no break).
- drop the duplicate plain-ASCII pass from extract_unicode_strings —
  extract_file_strings already runs extract_ascii_strings alongside,
  so every ASCII string was emitted twice.

Adds 3 regression tests (16 total, all passing).
@mnaza
mnaza force-pushed the fix/extractor-feature-corruption branch from 29ffe26 to bc3b51a Compare July 31, 2026 08:57
@mnaza

mnaza commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Corrected, now all 4 mergable

@marirs

marirs commented Aug 1, 2026

Copy link
Copy Markdown
Owner

conflicts..
if this is the latest - lets not fix the othe PR's

@marirs

marirs commented Aug 4, 2026

Copy link
Copy Markdown
Owner

still

@mnaza

mnaza commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Now should merge

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.

Silent feature corruption: registers parsed as numbers, u32-masked negatives on x64, duplicate stack-string and ASCII-string features

2 participants