Files
booth/persistent-memory.d/2026-09-24-upload-names-two-crashes-then-two-holes.md

2.9 KiB

Upload names: two crashes found, then two holes in the fix

2026-09-24. Commits 92c774e, 225ba32; heid bug-hunt thread 01M3AXG27KQDMA6P3RTAAYMEHP.

What was wrong

design-dev's r3 bug hunt (hulda) found that /upload returned a 500 for a multipart filename carrying a NUL: safe_upload_name stripped path, dots and length but not NUL, and (dest / name).open("wb") raised ValueError. The upload's except Exception tore the booth down and re-raised.

Fixing it turned up a second 500 on the same line: the cap was base[:200], 200 CHARACTERS. NAME_MAX is 255 BYTES, so 200 two-byte characters (é, or any CJK name) raised ENAMETOOLONG.

The first fix, and the two holes a single arm found in it

92c774e stripped NUL first, then applied the dot rule, then capped at 200 UTF-8 bytes via encode("utf-8", "surrogatepass")[:200].decode("utf-8", "ignore"), taking the cut out of the stem so the extension survived.

Heid's own review of that diff found nothing. hulda, reading the whole snapshot against the declared invariants, found:

  1. The surrogate was dropped LAST. The final decode("ignore") removed a lone surrogate after lstrip(".") had already run, so "\ud800.forever" came out as .forever, which is the keep marker, and "\ud800.." as ... The NUL had been moved first for exactly this reason; the same discipline was not applied to the other droppable class. It was not reachable over HTTP: Starlette decodes a multipart filename strictly (utf-8, else latin-1), so it never yields a lone surrogate. The fix made the helper right by construction anyway.
  2. A cut could manufacture a kind. A suffix too long to keep (>16 bytes) was cut like text, and the cut could land on a shorter suffix that means something: "a"*196 + ".png" + "x"*17 became ….png, an image.
  3. The 16-byte extension threshold was unguarded: every test suffix was 4 bytes, so <= 4 survived. A .jpeg case now pins it.

225ba32 does one pass first (NUL and everything unencodable), then basename and the dot rule, then the byte cap. A cut whose classify/doc_kind differs from the original's has its dots replaced with _. Falsifiers: tests/mutations/upload_names.toml, 7/7.

The lessons

  • A sanitiser drops everything droppable FIRST, then applies the structural rules. Anything dropped after a rule can defeat that rule.
  • A NUL test through httpx files= proves nothing. httpx percent-escapes the NUL, so the server sees a literal %00. Post a raw multipart body. The first integration test passed pre-fix for this reason.
  • Truncating by length can change a file's meaning. In the Booth the kind comes from the extension, so a cut has to preserve the kind, not only the byte count.
  • The single-arm hunt earned its cost. Heid's own read traced only the hunks; hulda read the declared invariants against the whole bundle.