Skip to content

Binary/RE detector: no rule for memcpy/memmove with a non-constant (wire-controlled) length #150

Description

@lyubomir-bozhinov

Summary

The Binary/RE decompiled-vuln detector (scripts/binary-vuln-bench.mjs) ships 6 rules — gets, strcpy, sprintf, format-string, system/popen injection, and malloc-multiply integer overflow — but has no rule for memcpy/memmove with a non-constant length. That is the classic unbounded-copy overflow, and it is a blind spot in exactly the pattern this project describes as its core target.

The reasoning layer already treats this as the textbook flaw:

  • src/orchestration/prompts.ts:60 — the worker "REFUSES the moment the code it is shown looks like a textbook flaw — e.g. a wire-controlled length flowing into an unchecked memcpy".
  • src/orchestration/prompts.ts:80 — "the smoking-gun line (e.g. the unchecked memcpy of a wire length)".
  • src/orchestration/index.ts:11 — example question: "Does this function validate the length parameter before the memcpy?".

So the LLM adjudication layer is built around this sink, but the fast static pre-filter can't surface it as a candidate at all. strcpy is covered; its memcpy(dst, src, len) sibling is not.

The detection gap

RULES in scripts/binary-vuln-bench.mjs:22-29 has no memcpy/memmove entry. A decompiled artifact like:

void parse(char *pkt) {
  char buf[128];
  unsigned len = ntohl(*(unsigned *)pkt);
  memcpy(buf, pkt + 4, len);   // wire-controlled length into a fixed buffer
}

produces zero hits.

Proposed rule

A discriminating rule — it must fire on a variable/wire-controlled length but stay silent when the length is a sizeof(...) expression or a numeric literal, otherwise it is a tautology (a bare /memcpy/ match would flag every safe bounded copy too):

{ id: 'B-MEMCPY',
  desc: 'memcpy()/memmove() with a non-constant length — candidate overflow from an unchecked/wire-controlled size',
  test: rx(/\b(?:memcpy|memmove)\s*\([^,]+,[^,]+,\s*(?!\d|sizeof\b)[^)\s][^)]*\)/) },

It inspects the third argument: fires on len / hdr->len / *p / ntohl(hdr->len) / n - 4, stays silent on sizeof(dst) / 16 / 0x40. Validated against a 22-case dangerous/safe matrix — perfect discrimination.

Scope (honest)

Same posture as the existing corpus note: this is static pattern detection over decompiled output — it surfaces a candidate sink, not a proven bug. It does not do taint analysis or prove the length is actually attacker-reachable (that is the reasoning layer's job). Constants are inlined in decompiled output, so a named-macro length isn't a concern by construction. Out of scope for this rule: additive/shift overflow precursors, strcat, exec-family injection — separate follow-ups.

RED-first plan

  1. Add a bad fixture (memcpy with a wire-decoded length, expect: ["B-MEMCPY"]) and a clean control with two safe memcpys (sizeof-bounded + constant length) to bench/binary-vulns/corpus.json. The clean control is what forces a real regex — a naive /memcpy/ rule false-positives on it and fails the discrimination gate.
  2. Add direct assertions to src/__tests__/binary-vuln-bench.test.ts. Both go RED with no rule present.
  3. Add the single RULES entry → GREEN.

I hit this reviewing the binary loadout and have the fix ready. Happy to take it if no one's on it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions