diff --git a/bench/binary-vulns/corpus.json b/bench/binary-vulns/corpus.json index cd7241b..f4a61b1 100644 --- a/bench/binary-vulns/corpus.json +++ b/bench/binary-vulns/corpus.json @@ -43,6 +43,20 @@ "expect": ["B-SPRINTF"], "content": "void fmt(char *input) {\n char b[16];\n sprintf(b, \"%s\", input);\n}\n" }, + { + "id": "memcpy-wire-length", + "desc": "memcpy with an attacker/wire-controlled length (unchecked) — buffer overflow", + "clean": false, + "expect": ["B-MEMCPY"], + "content": "void parse(char *pkt) {\n char buf[128];\n unsigned len = ntohl(*(unsigned *)pkt);\n memcpy(buf, pkt + 4, len);\n}\n" + }, + { + "id": "memcpy-bounded-control", + "desc": "Safe control — memcpy bounded by sizeof and by a constant length", + "clean": true, + "expect": [], + "content": "void copy(char *src) {\n char dst[64];\n memcpy(dst, src, sizeof(dst));\n char hdr[4];\n memcpy(hdr, src, 4);\n}\n" + }, { "id": "bounded-control", "desc": "Safe control — bounded strncpy/snprintf/fgets", diff --git a/scripts/binary-vuln-bench.mjs b/scripts/binary-vuln-bench.mjs index 1a07a72..70a98a7 100644 --- a/scripts/binary-vuln-bench.mjs +++ b/scripts/binary-vuln-bench.mjs @@ -26,6 +26,24 @@ export const RULES = [ { id: 'B-FORMAT-STRING', desc: 'printf(var) — user-controlled format string', test: rx(/\bf?printf\s*\(\s*[a-zA-Z_]\w*\s*\)/) }, { id: 'B-CMD-INJECTION', desc: 'system()/popen() on a variable — command injection', test: rx(/\b(system|popen)\s*\(\s*[a-zA-Z_]\w*/) }, { id: 'B-INT-OVERFLOW', desc: 'malloc/calloc with multiplication — integer-overflow alloc size', test: rx(/\b(malloc|calloc|alloca|realloc)\s*\([^)]*\*/) }, + // Inspects the length (3rd) arg: fires when it begins with a variable / field / + // deref / decoded value (`len`, `hdr->len`, `ntohl(hdr->len)`) and stays silent + // when it begins with a `sizeof` expression or a numeric literal. Two subtleties: + // the `[^)\s]` after the lookahead is load-bearing — without it `\s*` backtracks + // onto whitespace and bounded forms leak through; and args 1-2 are matched with a + // single level of BALANCED parens `(?:[^,()]|\([^()]*\))+` so a cast `(void *)dst` + // and a comma-bearing call `get(a,b)` are each consumed as ONE arg — neither + // mis-splits the list (the original naive `[^,]+` split on the comma inside a + // call and false-fired on a bounded length). Directional static pre-filter, not + // taint analysis — disclosed misses (silently NOT flagged): a computed length + // that starts with a digit (`8*count`) or with `sizeof` plus a term + // (`sizeof(x)+n`), and a 1st/2nd arg with a DOUBLY-nested paren + // (`wrap(get(a,b))`). The `__memcpy_chk`/`memcpy_s`/`wmemcpy` variants are out of + // scope (the `\b...\(` shape); the `_chk` form is usually the bounded, safe one. + // Comments aren't stripped: a mid-argument `/* ... */` obscuring a sizeof/literal + // length would false-fire, but decompiled output does not emit inline comments. + // Decompiled output inlines constants, so a named-macro length does not arise. + { 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*(?:[^,()]|\([^()]*\))+,\s*(?!\d|sizeof\b)[^)\s][^)]*\)/) }, ]; export function loadCorpus() { diff --git a/src/__tests__/binary-vuln-bench.test.ts b/src/__tests__/binary-vuln-bench.test.ts index 9b30304..d590882 100644 --- a/src/__tests__/binary-vuln-bench.test.ts +++ b/src/__tests__/binary-vuln-bench.test.ts @@ -32,9 +32,50 @@ describe('binary/RE decompiled-vuln benchmark', () => { expect(detect('printf(user);').has('B-FORMAT-STRING')).toBe(true); }); + it('flags memcpy/memmove with a non-constant length, not a bounded one', () => { + const fires = (c: string) => expect(detect(c).has('B-MEMCPY'), c).toBe(true); + const silent = (c: string) => expect(detect(c).has('B-MEMCPY'), c).toBe(false); + + // fires on a variable / field / deref / wire-decoded / cast length + fires('memcpy(buf, pkt + 4, len);'); + fires('memcpy(dst, src, hdr->len);'); + fires('memcpy(dst, src, ntohl(hdr->len));'); + fires('memmove(d, s, n);'); + fires('memcpy(&d, s, (size_t)len);'); + fires('memcpy(&local_38, param_1, size);'); + // pointer casts on args 1-2 are the common decompiled shape — must still fire + fires('memcpy((void *)dst, src, len);'); + fires('memmove((uint8_t *)dst, (uint8_t *)src, len);'); + fires('memcpy((char *)dst + 4, src, len);'); + // a comma-bearing call in an arg is consumed as one arg, so a non-constant + // length after it is still caught + fires('memcpy(get_dst(a,b), src, len);'); + + // silent when the length is sizeof-bounded or a numeric literal — this is what + // forces a discriminating rule rather than a bare /memcpy/ match + silent('memcpy(dst, src, sizeof(dst));'); + silent('memcpy(hdr, src, 4);'); + silent('memcpy(dst, src, 0x40);'); + silent('memcpy(dst, src, sizeof(struct hdr));'); + // a comma inside a nested call must NOT mis-split the arg list and false-fire + // on a bounded sizeof length; casts on args 1-2 must not either + silent('memcpy(dst, get_src(a,b), sizeof(dst));'); + silent('memcpy((void *)dst, (void *)src, sizeof(dst));'); + + // Disclosed directional limits (see the rule comment): these non-constant + // lengths are NOT flagged — pinned here so the scope boundary is auditable and + // any future tightening surfaces as a deliberate change, not a silent one. + silent('memcpy(d, s, 8 * count);'); // leading-digit computed length + silent('memcpy(d, s, sizeof(x) + n);'); // sizeof plus a variable term + silent('memcpy(dst, wrap(get(a,b)), len);'); // doubly-nested paren in an arg + silent('__memcpy_chk(dst, src, len, dn);'); // _chk variant (usually bounded) + silent('wmemcpy(dst, src, len);'); // wide-char variant + }); + it('ruleset spans memory-safety + injection + integer-overflow', () => { expect(RULES.length).toBeGreaterThanOrEqual(6); expect(RULES.some((r) => r.id === 'B-CMD-INJECTION')).toBe(true); expect(RULES.some((r) => r.id === 'B-INT-OVERFLOW')).toBe(true); + expect(RULES.some((r) => r.id === 'B-MEMCPY')).toBe(true); }); });