[AUTOMATED] fix(p1): symbolnamechars — a symbol name's raw bytes stop restructuring the C document they are printed into (DIV-94)

[mahaloz]

A symbol name reaches emitted C verbatim, and nothing between .strtab and the
printer validates one byte of it.

The name is printed into the // Function: <name> @ <addr> header comment, the
.h prototype, the definition, every call site and the .asm label. Three
shapes therefore restructure the C document rather than merely looking odd:

• */ closes the header comment early, so the @ 0x401020 after it becomes code;
• a raw 0x0a splits all five of those renderings across two lines each
  (void a\nb(void) declares nothing, and kuna functions prints its TSV row on
  two lines);
• // comments out the rest of the line, parameter list and ; included.

A fourth breaks identity rather than syntax: the name is decoded with
String::from_utf8_lossy, so two symbols differing only in an invalid byte
(a\x80b at one address, a\x81b at another) become the same String — the
export carries two definitions and two prototypes with one name, and no reader
can tell which call went where.

Symbol-name bytes are attacker-controlled data no header check validates, so all
four cost a few .strtab bytes.

The option

symbolnamechars, off|safe|ident, default safe, loader-tier
(KUNA_SYMBOLNAMECHARS env bridge, the symbolnamerepair pattern), exported by
both CLI surfaces.

value	behavior
off	verbatim — what the binary literally claims, for someone auditing exactly that
safe	default. The structural set only
ident	the full identifier reduction, per :: component

It is VALUED and not a bool, and that is the measured half. The most common
name in the wild that is not valid C is not hostile at all — it is gcc's clone
suffix (err_fatal.constprop.0, main.part.1, add_fdes.cold,
__x86.get_pc_thunk.bx), which appears on most -O2 binaries and on six
fixtures checked into this repo. safe is a measured no-op on every one of
them (see Collateral); ident rewrites them all, which is exactly why it must be
reachable and must not be the default.

The exact safe set

Applied to &[u8], before any UTF-8 decode. A byte is rewritten iff it is

• an ASCII control byte (0x00–0x1F, 0x7F) — covers \n, \r, \t, \0;
• ", ' or \;
• a * or / that forms */, /* or // with its neighbor (both
  characters of the pair are rewritten; a LONE * or / is left alone — it is
  not a comment delimiter, so rewriting it buys nothing structural and costs
  collateral);
• part of an invalid UTF-8 sequence.

Everything else — . $ @ : - + < > ( ) ; { } and all
valid multi-byte UTF-8 — is untouched. :: survives both modes, because the
scope splitter reads it. ident never DELETES a name either: its reduction drops
empty components, so a name made only of non-identifier characters (***) would
reduce to nothing and put every such symbol under one empty name — the very
collapse this option exists to prevent. Such a name is escaped whole instead
(_x2a_x2a_x2a), still an identifier and still injective.

Two deliberate deviations from the shape this was scoped with, both argued in
the module header:

1. // is in the set. It was not in the original list, but void a//b(void)
   comments out the parameter list and the ; — the same structural corruption
   the other entries exist to stop. No clone suffix, mangled symbol or Rust path
   contains //, so it costs no collateral.
2. Every rewritten byte becomes _x<hh>, not _. A single _ is not
   injective: a"b, a'b and a\nb all collapse to a_b, which reproduces the
   exact redefinition defect the invalid-UTF-8 half exists to fix, merely with
   a different trigger. One escape scheme for the whole set keeps the rewrite
   injective, and costs nothing legible because safe fires on no real name.

Mint time, not print time

kuna decompile <name>, the console's load function and the DB scope path all
key on the string in prog.symbols. A printer-side rewrite would put a name in
the .c that cannot be handed back to the CLI — for an agent-first tool, a worse
bug than the one being fixed. So the sanitizer runs where the name is minted:
after the demangler (it sees the reduced name, not the _ZN… envelope, and
ident does not fold the demangler's output twice) and before the scope splitter
(so it never contends with symbolnamerepair over the same empty component), at
both mints — the loader's .symtab/PLT/.dynsym/ET_REL and data-symbol walks in
kuna-analysis/src/loadimage_object.rs, and the analysis passes' recovered names
(DWARF DW_AT_name, Go pclntab, PDB, RTTI) via
pass.rs (AnalysisOutput::sanitize_names). One string then serves prog.symbols,
kuna functions, kuna decompile <name>, the .c, the .h and the .asm.

Two unflagged strict fixes ride along

• printc.rs's /* renamed from "<raw>" */ note interpolated a raw type name
  into a comment — the identical injection hole in the construct
  sanitize_type_name was written to close. It now escapes what it quotes. No
  flag: it only ever corrects wrong output.
• kuna_itaniumrtti.rs (sanitize_class_name) becomes a one-line wrapper over the
  hoisted shared sanitize_ident_chain, so ident and the RTTI class-name
  recovery are one implementation. The RTTI path stays unconditional — the gate
  decides only whether the loader's names go through it — and its six existing
  unit tests are kept as the regression net for the hoist.

Witness — decompile-project on the new fixture, verbatim

hostile_symname_x86_64: a 5 KB x86-64 ELF built reproducibly from checked-in
assembly plus a checked-in .strtab patch script (GNU as will not put a raw
0x0a, 0x80 or 0x81 inside a symbol name, so those three names are declared
as same-length ASCII placeholders and patched; the */, // and d*/x names
assemble verbatim). Six symbols, one per half of the defect.

Before (--option symbolnamechars off == today's main):

#include "hostile_symname_x86_64.h"

// Function: main @ 0x401000
unsigned int main(void)
{
  a*/b();
  a//b();
  a
b();
  a<U+FFFD>b();
  a<U+FFFD>b();
  return d*/x;
}

// Function: a*/b @ 0x401020
unsigned long a*/b(void)
{
  return 1;
}

// Function: a//b @ 0x401026
unsigned long a//b(void)
{
  return 2;
}

// Function: a
b @ 0x40102c
unsigned long a
b(void)
{
  return 3;
}

// Function: a<U+FFFD>b @ 0x401032
unsigned long a<U+FFFD>b(void)
{
  return 4;
}

// Function: a<U+FFFD>b @ 0x401038
unsigned long a<U+FFFD>b(void)        <-- C redefinition
{
  return 5;
}

/* function prototypes */          (the .h)
unsigned int main(void);
unsigned long a*/b(void);
unsigned long a//b(void);
unsigned long a
b(void);
unsigned long a<U+FFFD>b(void);
unsigned long a<U+FFFD>b(void);    <-- duplicate prototype

6:main:  ; 0x401000               (the .asm labels)
15:a*/b:  ; 0x401020
19:a//b:  ; 0x401026
24:b:  ; 0x40102c                 <-- the label lost its first line
28:a<U+FFFD>b:  ; 0x401032
32:a<U+FFFD>b:  ; 0x401038
38:d*/x:  ; 0x402000

After (the shipped default):

#include "hostile_symname_x86_64.h"

// Function: main @ 0x401000
unsigned int main(void)
{
  a_x2a_x2fb();
  a_x2f_x2fb();
  a_x0ab();
  a_x80b();
  a_x81b();
  return d_x2a_x2fx;
}

// Function: a_x2a_x2fb @ 0x401020
unsigned long a_x2a_x2fb(void)
{
  return 1;
}

// Function: a_x2f_x2fb @ 0x401026
unsigned long a_x2f_x2fb(void)
{
  return 2;
}

// Function: a_x0ab @ 0x40102c
unsigned long a_x0ab(void)
{
  return 3;
}

// Function: a_x80b @ 0x401032
unsigned long a_x80b(void)
{
  return 4;
}

// Function: a_x81b @ 0x401038
unsigned long a_x81b(void)
{
  return 5;
}

/* function prototypes */          (the .h)
unsigned int main(void);
unsigned long a_x2a_x2fb(void);
unsigned long a_x2f_x2fb(void);
unsigned long a_x0ab(void);
unsigned long a_x80b(void);
unsigned long a_x81b(void);

6:main:  ; 0x401000               (the .asm labels)
15:a_x2a_x2fb:  ; 0x401020
19:a_x2f_x2fb:  ; 0x401026
23:a_x0ab:  ; 0x40102c
27:a_x80b:  ; 0x401032
31:a_x81b:  ; 0x401038
37:d_x2a_x2fx:  ; 0x402000

Six distinct names, and the round-trip holds — every name kuna functions prints
resolves through kuna decompile <name>:

$ for n in $(kuna functions ./hostile_symname_x86_64 | cut -f2); do kuna decompile ./hostile_symname_x86_64 "$n" | head -1; done
unsigned int main(void)
unsigned long a_x2a_x2fb(void)
unsigned long a_x2f_x2fb(void)
unsigned long a_x0ab(void)
unsigned long a_x80b(void)
unsigned long a_x81b(void)

Collateral

decompile-all --json is byte-identical on 17 real binaries, measured twice
over — base-vs-branch (the whole PR, printc included) and off-vs-default (the
option alone):

binary	base vs branch	off vs default
tests/bug-repro/grep	IDENTICAL	IDENTICAL
tests/bug-repro/sort	IDENTICAL	IDENTICAL
tests/bug-repro/faillog	IDENTICAL	IDENTICAL
tests/bug-repro/libselinux.so.1	IDENTICAL	IDENTICAL
tests/bug-repro/betaflight_STM32F405.elf (ARM Cortex-M)	IDENTICAL	IDENTICAL
tests/fixtures/fid/prog	IDENTICAL	IDENTICAL
cpp_mangled_x86_64 (C++)	IDENTICAL	IDENTICAL
cppsig_x86_64.so (C++)	IDENTICAL	IDENTICAL
anon_namespace_x86_64 (C++)	IDENTICAL	IDENTICAL
cpp_noreturn_x86_64 (C++)	IDENTICAL	IDENTICAL
noreturn_error_x86_64	IDENTICAL	IDENTICAL
regglobal_fmt_x86_64	IDENTICAL	IDENTICAL
mcount_x86_64	IDENTICAL	IDENTICAL
plt_ppc64le	IDENTICAL	IDENTICAL
i386_pie_nl	IDENTICAL	IDENTICAL
pe_dwarf.exe (PE)	IDENTICAL	IDENTICAL
hostile_scope_x86_64	IDENTICAL	IDENTICAL

Does safe rewrite ANY name on ANY real binary? No — and that is proved, not
assumed. kuna functions off-vs-default over every one of the 92 tracked
ELF/PE/Mach-O files in the repo rewrites 0 names:

$ for each tracked object file: diff <(kuna functions $f --option symbolnamechars off) <(kuna functions $f)
scanned 92 object files; 0 had a name rewritten by safe

The harness is not vacuous — pointed at the new hostile fixture (untracked at
scan time) it reports all five function names plus the data symbol as changed.
That covers the six clone-suffix fixtures the scout flagged
(err_fatal.constprop.0, main.part.1, add_fdes.cold, .L10,
__x86.get_pc_thunk.bx, 0000001b.plt_call.printf@@GLIBC_2.17, .ctors.65535).
ident rewrites every one of them, e.g. on noreturn_error_x86_64:

--- default (safe) ---                      --- ident ---
// Function: err_warn.constprop.0 @ ...     // Function: err_warn_constprop_0 @ ...
unsigned long err_warn.constprop.0(void)    unsigned long err_warn_constprop_0(void)
  err_fatal.constprop.0(); // no-return       err_fatal_constprop_0(); // no-return

Speed

Interleaved, alternating arms, min-of-4 decompile-all --json. The box was
running several other agents concurrently, which is why the spread goes both
directions and why min-of-N over interleaved runs is the only honest form here.

binary	base min	branch min	delta
grep	15.009 s	14.291 s	-4.78%
libselinux.so.1	9.103 s	9.227 s	+1.36%
regglobal_fmt_x86_64	2.608 s	2.588 s	-0.76%

All inside the 5% budget, and the sign flips between binaries — this is noise,
and structurally it has to be: safe's pre-scan returns a borrow and allocates
nothing for a clean name, and the gate is read once per symbol walk rather than
once per symbol.

Tests

• tests/stages/kuna-symbolnamechars.xml — 9 assertions over the new fixture
  (the */ name, the newline name, both invalid-UTF-8 names as distinct
  spellings, a call site, the data symbol, and two min=0 max=0 guards that no
  comment delimiter survives anywhere). DEFAULT arm only, and that is a real
  limit rather than an oversight: symbolnamechars is consumed inside
  load file, and a script's option requires a live image, so no stage command
  can set the mode before the load it governs. Same limit, same reason, as
  tests/stages/kuna-symbolnamerepair.xml (fix(analysis): C++ anonymous namespaces stop aborting the load, and symbolnamerepair backstops the rest (DIV-88) #335) — cited there too.
• decompiler/crates/kuna-console/tests/verify_symbolnamechars.rs — the
  three-way coverage the XML cannot carry, 4 e2e tests: off restores every half
  including the two-symbols-one-String collapse; safe neutralizes all four and
  keeps six symbols six names; safe is byte-identical to off on a real gcc
  -O2 binary while ident folds its .constprop.0; and the name-keyed-lookup
  risk is pinned — every name function_entries reports still resolves through
  lookup_symbol (what kuna decompile <name> and load function go through) on
  the hostile fixture and on two real unstripped C++ binaries.
• 10 sanitizer unit tests (the exact safe set, the no-op-on-real-names list, the
  distinctness properties, ident never deleting a name, the env gate, the
  hoisted ident chain) plus the RTTI class-name tests kept unchanged across the
  hoist, plus a new printc test that the /* renamed from "…" */ note cannot be
  escaped by the raw name.

Gates

make test          675/675 assertions passed -- PARITY OK   (no re-pin)
make test-stages   547/547 assertions passed -- PARITY OK
make rust-test     324 test groups, 4849 passed, 0 failed, 38 ignored (exit 0)
make check-spec    check-spec OK (lenient mode) / check-spec OK (strict mode)
kuna catalog --check   catalog OK: documents exactly the registered kuna options

Bookkeeping

• Catalog 119 -> 120 settables (analysis tier 43 -> 44); catalog JSON row
  commas 118 -> 119; catalog_bytecompat fixture counts 119 -> 120 (x3)
  and phase_catalog.json regenerated.
• Stages corpus file count 215 -> 216 (kuna-base/src/xml.rs);
  docs/baseline-stages.json re-recorded 538 -> 547 keys.
• DIV-90 in docs/history.md. The highest on main is DIV-89 and siblings may
  claim 90/91, so this row and its two references in
  architecture.rs/kuna_phases/tests.rs may need renumbering at merge.

Closes #340

🤖 Generated with Claude Code

https://claude.ai/code/session_011KpG7qK6BDFZyPnyo4r1c1