fix(upload): drop what no name can hold BEFORE the dot rule; a cut never manufactures a kind

Heid bug hunt, hulda, second round on 92c774e:

- A lone surrogate was dropped at the final decode, after the leading-dot
  rule had already run, so "\ud800.forever" came out as .forever, the
  keep marker, and "\ud800.." as "..". The NUL and every unencodable
  character now go first, in one pass, so nothing dropped later can shield
  a dot. Starlette decodes a multipart filename strictly (utf-8, else
  latin-1), so this was not reachable over HTTP; the helper is now right by
  construction regardless.
- A suffix too long to keep was cut like text, and the cut could land on a
  shorter suffix that means something: "….png" out of "….pngxxxx…"
  became an image. A cut that changes classify/doc_kind now has its dots
  neutralised.
- The 16-byte extension threshold was unguarded (every test suffix was 4
  bytes); a .jpeg case pins it.

Falsifiers: tests/mutations/upload_names.toml, 7/7 proved. Not taken here,
as they sit in the upload route rather than this helper: the pickup-id
mkdir outside the try (a FileExistsError race), rmtree(ignore_errors)
hiding a failed cleanup, and a CancelledError skipping cleanup.
This commit is contained in:
vh
2026-09-24 17:04:35 -07:00
parent 92c774e105
commit 225ba32209
3 changed files with 87 additions and 25 deletions
+23 -14
View File
@@ -987,28 +987,37 @@ UPLOAD_NAME_MAX_BYTES = 200 # NAME_MAX is 255 bytes; the rest is _dedupe_name's
def safe_upload_name(name: str, fallback: str) -> str: def safe_upload_name(name: str, fallback: str) -> str:
"""Reduce a client-supplied filename to a safe basename (no path, no hidden) """Reduce a client-supplied filename to a safe basename (no path, no hidden)
that the filesystem can actually hold. that the filesystem can actually hold, and whose kind is the one it came in
with.
Two names used to reach `open()` and raise, a 500 with the booth torn down Two names used to reach `open()` and raise, a 500 with the booth torn down
(r3 heid bug hunt, hulda): a NUL, the one byte no POSIX name can hold (r3 heid bug hunt, hulda): a NUL, the one byte no POSIX name can hold
(ValueError), and a name over NAME_MAX, which is 255 BYTES — a 200-character (ValueError), and a name over NAME_MAX, which is 255 BYTES — a 200-character
cap let 200 two-byte characters through (ENAMETOOLONG). The NUL goes FIRST, cap let 200 two-byte characters through (ENAMETOOLONG).
so it cannot shield a leading dot from the hide rule. The cap is 200 UTF-8
bytes, leaving room for `_dedupe_name`'s suffix, and it comes out of the ORDER IS THE POINT. Everything no filename can hold goes FIRST — the NUL,
STEM: the extension is what `classify` reads, so a cut `.png` would no and a lone surrogate, which no UTF-8 name can encode — so nothing dropped
longer be an image. A cut through a multibyte character drops the partial later can shield a leading dot from the hide rule: dropped last, the
character, and a lone surrogate, which no filename can encode either, goes surrogate in "\\ud800.forever" left `.forever`, which is the keep marker
the same way. (hulda, second round). Then the basename and the dot rule, then the cap:
200 UTF-8 bytes, leaving room for `_dedupe_name`'s suffix, taken out of the
STEM so the extension `classify` reads survives, and never through the middle
of a character. A suffix too long to be an extension is cut like any other
text, and a cut that lands on a SHORTER suffix meaning something else
(`….png` out of `….pngxxxx…`) has its dots neutralised, so the kind is never
manufactured.
""" """
base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip() base = (name or "").replace("\x00", "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")
base = base.replace("\\", "/").split("/")[-1].strip()
base = base.lstrip(".") # a leading dot would hide the file from every listing base = base.lstrip(".") # a leading dot would hide the file from every listing
stem, dot, ext = base.rpartition(".") stem, dot, ext = base.rpartition(".")
tail = dot + ext if stem and len((dot + ext).encode("utf-8", "surrogatepass")) <= 16 else "" tail = dot + ext if stem and len((dot + ext).encode("utf-8")) <= 16 else ""
head = stem if tail else base head = stem if tail else base
room = UPLOAD_NAME_MAX_BYTES - len(tail.encode("utf-8", "surrogatepass")) room = UPLOAD_NAME_MAX_BYTES - len(tail.encode("utf-8"))
base = (head.encode("utf-8", "surrogatepass")[:room] cut = head.encode("utf-8")[:room].decode("utf-8", "ignore") + tail
+ tail.encode("utf-8", "surrogatepass")).decode("utf-8", "ignore") if (classify(cut), doc_kind(cut)) != (classify(base), doc_kind(base)):
return base or fallback cut = cut.replace(".", "_")
return cut or fallback
def _dedupe_name(name: str, used: set) -> str: def _dedupe_name(name: str, used: set) -> str:
+47 -11
View File
@@ -1,7 +1,10 @@
# Upload filenames the filesystem cannot hold. Two reached open() and raised, # Upload filenames the filesystem cannot hold, and names whose kind a cut could
# a 500 with the booth torn down (r3 heid bug hunt, hulda, 2026-09-24): a NUL, # change. Two reached open() and raised, a 500 with the booth torn down (r3
# and a name over NAME_MAX (255 BYTES) that a 200-CHARACTER cap let through. # heid bug hunt, hulda, 2026-09-24): a NUL, and a name over NAME_MAX (255
# Every row is a change tests/test_booth.py claims to forbid. # BYTES) that a 200-CHARACTER cap let through. The second round (hulda) found a
# surrogate dropped after the dot rule (`.forever`, the keep marker) and a cut
# that manufactured `.png`. Every row is a change tests/test_booth.py claims to
# forbid.
unit = "upload names the filesystem can hold" unit = "upload names the filesystem can hold"
@@ -10,30 +13,44 @@ label = "a NUL in an upload name reaches open() (ValueError, a 500)"
file = "booth/app.py" file = "booth/app.py"
test = "tests/test_booth.py::test_upload_a_nul_in_a_filename_never_500s" test = "tests/test_booth.py::test_upload_a_nul_in_a_filename_never_500s"
old = ''' old = '''
base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip()''' base = (name or "").replace("\x00", "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")'''
new = ''' new = '''
base = (name or "").replace("\\", "/").split("/")[-1].strip()''' base = (name or "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")'''
[[mutation]] [[mutation]]
label = "the NUL is stripped after the dot rule (a NUL shields a leading dot)" label = "the NUL is stripped after the dot rule (a NUL shields a leading dot)"
file = "booth/app.py" file = "booth/app.py"
test = "tests/test_booth.py::test_safe_upload_name_drops_nul_before_the_dot_rule" test = "tests/test_booth.py::test_safe_upload_name_drops_nul_before_the_dot_rule"
old = ''' old = '''
base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip() base = (name or "").replace("\x00", "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")
base = base.replace("\\", "/").split("/")[-1].strip()
base = base.lstrip(".") # a leading dot would hide the file from every listing''' base = base.lstrip(".") # a leading dot would hide the file from every listing'''
new = ''' new = '''
base = (name or "").replace("\\", "/").split("/")[-1].strip() base = (name or "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")
base = base.replace("\\", "/").split("/")[-1].strip()
base = base.lstrip(".").replace("\x00", "") # a leading dot would hide the file from every listing''' base = base.lstrip(".").replace("\x00", "") # a leading dot would hide the file from every listing'''
[[mutation]]
label = "a lone surrogate is dropped after the dot rule (`.forever`, the keep marker, comes out)"
file = "booth/app.py"
test = "tests/test_booth.py::test_safe_upload_name_drops_every_unencodable_character_before_the_dot_rule"
old = '''
base = (name or "").replace("\x00", "").encode("utf-8", "surrogatepass").decode("utf-8", "ignore")
base = base.replace("\\", "/").split("/")[-1].strip()
base = base.lstrip(".") # a leading dot would hide the file from every listing'''
new = '''
base = (name or "").replace("\x00", "")
base = base.replace("\\", "/").split("/")[-1].strip()
base = base.lstrip(".").encode("utf-8", "surrogatepass").decode("utf-8", "ignore") # a leading dot would hide the file from every listing'''
[[mutation]] [[mutation]]
label = "the cap counts characters, not bytes (ENAMETOOLONG, a 500)" label = "the cap counts characters, not bytes (ENAMETOOLONG, a 500)"
file = "booth/app.py" file = "booth/app.py"
test = "tests/test_booth.py::test_upload_a_name_over_name_max_in_bytes_never_500s" test = "tests/test_booth.py::test_upload_a_name_over_name_max_in_bytes_never_500s"
old = ''' old = '''
base = (head.encode("utf-8", "surrogatepass")[:room] cut = head.encode("utf-8")[:room].decode("utf-8", "ignore") + tail'''
+ tail.encode("utf-8", "surrogatepass")).decode("utf-8", "ignore")'''
new = ''' new = '''
base = base[:200]''' cut = head[:room] + tail'''
[[mutation]] [[mutation]]
label = "the cut comes out of the whole name (a long .png stops being an image)" label = "the cut comes out of the whole name (a long .png stops being an image)"
@@ -43,3 +60,22 @@ old = '''
head = stem if tail else base''' head = stem if tail else base'''
new = ''' new = '''
head, tail = base, ""''' head, tail = base, ""'''
[[mutation]]
label = "only a 4-byte extension survives the cut (`.jpeg` is lost)"
file = "booth/app.py"
test = "tests/test_booth.py::test_safe_upload_name_keeps_the_extension_through_the_cut"
old = '''
tail = dot + ext if stem and len((dot + ext).encode("utf-8")) <= 16 else ""'''
new = '''
tail = dot + ext if stem and len((dot + ext).encode("utf-8")) <= 4 else ""'''
[[mutation]]
label = "a cut may land on a shorter suffix and manufacture a kind (`….png` out of `….pngxxx…`)"
file = "booth/app.py"
test = "tests/test_booth.py::test_safe_upload_name_never_manufactures_a_kind"
old = '''
if (classify(cut), doc_kind(cut)) != (classify(base), doc_kind(base)):
cut = cut.replace(".", "_")'''
new = '''
'''
+17
View File
@@ -366,11 +366,28 @@ def test_safe_upload_name_keeps_the_extension_through_the_cut():
# name that USED to fit (80 CJK characters, 240 bytes) must not lose its kind # name that USED to fit (80 CJK characters, 240 bytes) must not lose its kind
assert safe_upload_name("é" * 200 + ".png", "fb") == "é" * 98 + ".png" assert safe_upload_name("é" * 200 + ".png", "fb") == "é" * 98 + ".png"
assert classify(safe_upload_name("画" * 80 + ".png", "fb")) == "image" assert classify(safe_upload_name("画" * 80 + ".png", "fb")) == "image"
# a 5-byte extension is kept as well as a 4-byte one
assert safe_upload_name("é" * 200 + ".jpeg", "fb") == "é" * 97 + ".jpeg"
# an "extension" too long to be one is cut like any other text # an "extension" too long to be one is cut like any other text
long_ext = safe_upload_name("a." + "é" * 150, "fb") long_ext = safe_upload_name("a." + "é" * 150, "fb")
assert len(long_ext.encode("utf-8")) <= 200 and long_ext.startswith("a.é") assert len(long_ext.encode("utf-8")) <= 200 and long_ext.startswith("a.é")
def test_safe_upload_name_never_manufactures_a_kind():
# a cut through a long suffix can land on a SHORTER one: `.pngxxx…` is not
# an image, and its cut `….png` would be (heid bug hunt, hulda)
name = safe_upload_name("a" * 196 + ".png" + "x" * 17, "fb")
assert classify(name) == "other" and doc_kind(name) is None
assert name == "a" * 196 + "_png"
def test_safe_upload_name_drops_every_unencodable_character_before_the_dot_rule():
# a lone surrogate is dropped too, and it must go FIRST like the NUL: dropped
# last, it shielded the dot and `.forever` came out, which is the keep marker
assert safe_upload_name("\ud800.forever", "fb") == "forever"
assert safe_upload_name("\ud800..", "fb") == "fb"
def _upload(client, files): def _upload(client, files):
return client.post("/upload", files=files, follow_redirects=False) return client.post("/upload", files=files, follow_redirects=False)