From 92c774e105aedd5725ebc8e4202a24bb7601810a Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Thu, 24 Sep 2026 16:53:01 -0700 Subject: [PATCH] fix(upload): a NUL or an over-long name never reaches open() safe_upload_name let two names through that the filesystem cannot hold, and each raised at open(): a 500 with the booth torn down. A NUL raised ValueError, and a 200-character cap let 200 two-byte characters overrun NAME_MAX (255 bytes, ENAMETOOLONG). The NUL is now removed first, so it cannot shield a leading dot from the hide rule. The cap is 200 UTF-8 bytes, cut on a character boundary, and it comes out of the stem: the extension is what classify reads, so a name that used to fit (80 CJK characters) keeps its kind. The NUL test posts a raw multipart body: httpx percent-escapes a NUL in files=, so the server would see a literal %00 and the test would prove nothing. Falsifiers in tests/mutations/upload_names.toml, 4/4 proved. Found by design-dev's r3 heid bug hunt (hulda). --- booth/app.py | 28 +++++++++++++++-- tests/mutations/upload_names.toml | 45 +++++++++++++++++++++++++++ tests/test_booth.py | 51 +++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 tests/mutations/upload_names.toml diff --git a/booth/app.py b/booth/app.py index d78dc33..65605e9 100644 --- a/booth/app.py +++ b/booth/app.py @@ -982,11 +982,33 @@ def _form_text(form, key: str) -> str: return value if isinstance(value, str) else "" +UPLOAD_NAME_MAX_BYTES = 200 # NAME_MAX is 255 bytes; the rest is _dedupe_name's room + + def safe_upload_name(name: str, fallback: str) -> str: - """Reduce a client-supplied filename to a safe basename (no path, no hidden).""" - base = (name or "").replace("\\", "/").split("/")[-1].strip() + """Reduce a client-supplied filename to a safe basename (no path, no hidden) + that the filesystem can actually hold. + + 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 + (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, + 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 + STEM: the extension is what `classify` reads, so a cut `.png` would no + longer be an image. A cut through a multibyte character drops the partial + character, and a lone surrogate, which no filename can encode either, goes + the same way. + """ + base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip() base = base.lstrip(".") # a leading dot would hide the file from every listing - return base[:200] or fallback + stem, dot, ext = base.rpartition(".") + tail = dot + ext if stem and len((dot + ext).encode("utf-8", "surrogatepass")) <= 16 else "" + head = stem if tail else base + room = UPLOAD_NAME_MAX_BYTES - len(tail.encode("utf-8", "surrogatepass")) + base = (head.encode("utf-8", "surrogatepass")[:room] + + tail.encode("utf-8", "surrogatepass")).decode("utf-8", "ignore") + return base or fallback def _dedupe_name(name: str, used: set) -> str: diff --git a/tests/mutations/upload_names.toml b/tests/mutations/upload_names.toml new file mode 100644 index 0000000..36bdbc3 --- /dev/null +++ b/tests/mutations/upload_names.toml @@ -0,0 +1,45 @@ +# Upload filenames the filesystem cannot hold. Two reached open() and raised, +# a 500 with the booth torn down (r3 heid bug hunt, hulda, 2026-09-24): a NUL, +# and a name over NAME_MAX (255 BYTES) that a 200-CHARACTER cap let through. +# Every row is a change tests/test_booth.py claims to forbid. + +unit = "upload names the filesystem can hold" + +[[mutation]] +label = "a NUL in an upload name reaches open() (ValueError, a 500)" +file = "booth/app.py" +test = "tests/test_booth.py::test_upload_a_nul_in_a_filename_never_500s" +old = ''' + base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip()''' +new = ''' + base = (name or "").replace("\\", "/").split("/")[-1].strip()''' + +[[mutation]] +label = "the NUL is stripped after the dot rule (a NUL shields a leading dot)" +file = "booth/app.py" +test = "tests/test_booth.py::test_safe_upload_name_drops_nul_before_the_dot_rule" +old = ''' + base = (name or "").replace("\x00", "").replace("\\", "/").split("/")[-1].strip() + base = base.lstrip(".") # a leading dot would hide the file from every listing''' +new = ''' + base = (name or "").replace("\\", "/").split("/")[-1].strip() + base = base.lstrip(".").replace("\x00", "") # a leading dot would hide the file from every listing''' + +[[mutation]] +label = "the cap counts characters, not bytes (ENAMETOOLONG, a 500)" +file = "booth/app.py" +test = "tests/test_booth.py::test_upload_a_name_over_name_max_in_bytes_never_500s" +old = ''' + base = (head.encode("utf-8", "surrogatepass")[:room] + + tail.encode("utf-8", "surrogatepass")).decode("utf-8", "ignore")''' +new = ''' + base = base[:200]''' + +[[mutation]] +label = "the cut comes out of the whole name (a long .png stops being an image)" +file = "booth/app.py" +test = "tests/test_booth.py::test_safe_upload_name_keeps_the_extension_through_the_cut" +old = ''' + head = stem if tail else base''' +new = ''' + head, tail = base, ""''' diff --git a/tests/test_booth.py b/tests/test_booth.py index 9cb005a..aabeac0 100644 --- a/tests/test_booth.py +++ b/tests/test_booth.py @@ -344,6 +344,33 @@ def test_safe_upload_name(): assert safe_upload_name("...", "fb") == "fb" +def test_safe_upload_name_drops_nul_before_the_dot_rule(): + # NUL is the one byte no POSIX filename can hold; open() raises ValueError on it + assert safe_upload_name("a\x00b.png", "fb") == "ab.png" + assert safe_upload_name("\x00", "fb") == "fb" + # stripped FIRST, so a NUL cannot shield a leading dot from the hide rule + assert safe_upload_name("\x00.hidden", "fb") == "hidden" + + +def test_safe_upload_name_caps_bytes_not_characters(): + # NAME_MAX is 255 BYTES: 200 two-byte characters are 400 of them + name = safe_upload_name("é" * 200, "fb") + assert len(name.encode("utf-8")) <= 200 + assert name == "é" * 100 + # a cut through a multibyte character drops the partial character, never mangles it + assert safe_upload_name("a" + "é" * 150, "fb") == "a" + "é" * 99 + + +def test_safe_upload_name_keeps_the_extension_through_the_cut(): + # the cut comes out of the stem: a cut `.png` is no longer an image, and a + # 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 classify(safe_upload_name("画" * 80 + ".png", "fb")) == "image" + # an "extension" too long to be one is cut like any other text + long_ext = safe_upload_name("a." + "é" * 150, "fb") + assert len(long_ext.encode("utf-8")) <= 200 and long_ext.startswith("a.é") + + def _upload(client, files): return client.post("/upload", files=files, follow_redirects=False) @@ -385,6 +412,30 @@ def test_upload_sanitizes_traversal(client): assert not (data.parent / "passwd").exists() # nothing escaped upward +def test_upload_a_nul_in_a_filename_never_500s(client): + # A RAW body: httpx percent-escapes a NUL in `files=` (the server then sees + # a literal "%00" and the test proves nothing). A NUL reached open() and + # raised ValueError, a 500 with the booth torn down. + c, data = client + body = (b'--XyZ\r\nContent-Disposition: form-data; name="files"; filename="a\x00b.png"\r\n' + b"Content-Type: image/png\r\n\r\npng\r\n--XyZ--\r\n") + r = c.post("/upload", content=body, follow_redirects=False, + headers={"content-type": "multipart/form-data; boundary=XyZ"}) + assert r.status_code == 303, r.text + booth = data / r.headers["location"].split("/b/")[1].rstrip("/") + assert (booth / "ab.png").read_bytes() == b"png" + + +def test_upload_a_name_over_name_max_in_bytes_never_500s(client): + # 200 two-byte characters pass a 200-CHARACTER cap and overrun NAME_MAX + # (255 bytes): ENAMETOOLONG at open(), a 500 with the booth torn down + c, data = client + r = _upload(c, [("files", ("é" * 200 + ".txt", b"long", "text/plain"))]) + assert r.status_code == 303, r.text + booth = data / r.headers["location"].split("/b/")[1].rstrip("/") + assert (booth / ("é" * 98 + ".txt")).read_bytes() == b"long" + + def test_upload_rejects_too_many_files(tmp_path): app = create_app(tmp_path, start_sweeper=False, max_files=2) c = TestClient(app)