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).
This commit is contained in:
@@ -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, ""'''
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user