security: fix #7 — reject Windows reserved device names in template
render_filename_template's sanitization stripped reserved chars, control codes, whitespace, and `..` prefixes — but did not catch Windows reserved device names (CON, PRN, AUX, NUL, COM1-9, LPT1-9). On Windows, opening `con.jpg` for writing redirects to the CON device, so a tag value of `con` from a hostile booru would silently break Save to Library. Adds a frozenset of reserved stems and prefixes the rendered name with `_` if its lowercased stem matches. The check runs unconditionally (not Windows-gated) so a library saved on Linux can be copied to a Windows machine without breaking on these filenames. Audit-Ref: SECURITY_AUDIT.md finding #7 Severity: Low
This commit is contained in:
parent
b8cb47badb
commit
6ff1f726d4
@ -15,6 +15,18 @@ if TYPE_CHECKING:
|
||||
APPNAME = "booru-viewer"
|
||||
IS_WINDOWS = sys.platform == "win32"
|
||||
|
||||
# Windows reserved device names (audit finding #7). Filenames whose stem
|
||||
# (before the first dot) lower-cases to one of these are illegal on
|
||||
# Windows because the OS routes opens of `con.jpg` to the CON device.
|
||||
# Checked by render_filename_template() unconditionally so a library
|
||||
# saved on Linux can still be copied to a Windows machine without
|
||||
# breaking on these stems.
|
||||
_WINDOWS_RESERVED_NAMES = frozenset({
|
||||
"con", "prn", "aux", "nul",
|
||||
*{f"com{i}" for i in range(1, 10)},
|
||||
*{f"lpt{i}" for i in range(1, 10)},
|
||||
})
|
||||
|
||||
|
||||
def hypr_rules_enabled() -> bool:
|
||||
"""Whether the in-code hyprctl dispatches that change window state
|
||||
@ -291,6 +303,16 @@ def render_filename_template(template: str, post: "Post", ext: str) -> str:
|
||||
if len(rendered) > 200:
|
||||
rendered = rendered[:200].rstrip("._ ")
|
||||
|
||||
# Reject Windows reserved device names (audit finding #7). On Windows,
|
||||
# opening `con.jpg` or `prn.png` for writing redirects to the device,
|
||||
# so a tag value of `con` from a hostile booru would silently break
|
||||
# save. Prefix with `_` to break the device-name match while keeping
|
||||
# the user's intended name visible.
|
||||
if rendered:
|
||||
stem_lower = rendered.split(".", 1)[0].lower()
|
||||
if stem_lower in _WINDOWS_RESERVED_NAMES:
|
||||
rendered = "_" + rendered
|
||||
|
||||
if not rendered:
|
||||
return f"{post.id}{ext}"
|
||||
|
||||
|
||||
@ -89,3 +89,57 @@ def test_data_dir_tightens_loose_existing_perms(tmp_path, monkeypatch):
|
||||
config.data_dir()
|
||||
mode = os.stat(pre).st_mode & 0o777
|
||||
assert mode == 0o700
|
||||
|
||||
|
||||
# -- render_filename_template Windows reserved names (finding #7) --
|
||||
|
||||
|
||||
def _fake_post(tag_categories=None, **overrides):
|
||||
"""Build a minimal Post-like object suitable for render_filename_template.
|
||||
|
||||
A real Post needs file_url + tag_categories; defaults are fine for the
|
||||
reserved-name tests since they only inspect the artist/character tokens.
|
||||
"""
|
||||
from booru_viewer.core.api.base import Post
|
||||
return Post(
|
||||
id=overrides.get("id", 999),
|
||||
file_url=overrides.get("file_url", "https://x.test/abc.jpg"),
|
||||
preview_url=None,
|
||||
tags="",
|
||||
score=0,
|
||||
rating=None,
|
||||
source=None,
|
||||
tag_categories=tag_categories or {},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("reserved", [
|
||||
"con", "CON", "prn", "PRN", "aux", "AUX", "nul", "NUL",
|
||||
"com1", "COM1", "com9", "lpt1", "LPT1", "lpt9",
|
||||
])
|
||||
def test_render_filename_template_prefixes_reserved_names(reserved):
|
||||
"""A tag whose value renders to a Windows reserved device name must
|
||||
be prefixed with `_` so the resulting filename can't redirect to a
|
||||
device on Windows. Audit finding #7."""
|
||||
post = _fake_post(tag_categories={"Artist": [reserved]})
|
||||
out = config.render_filename_template("%artist%", post, ext=".jpg")
|
||||
# Stem (before extension) must NOT be a reserved name.
|
||||
stem = out.split(".", 1)[0]
|
||||
assert stem.lower() != reserved.lower()
|
||||
assert stem.startswith("_")
|
||||
|
||||
|
||||
def test_render_filename_template_passes_normal_names_unchanged():
|
||||
"""Non-reserved tags must NOT be prefixed."""
|
||||
post = _fake_post(tag_categories={"Artist": ["miku"]})
|
||||
out = config.render_filename_template("%artist%", post, ext=".jpg")
|
||||
assert out == "miku.jpg"
|
||||
|
||||
|
||||
def test_render_filename_template_reserved_with_extension_in_template():
|
||||
"""`con.jpg` from a tag-only stem must still be caught — the dot in
|
||||
the stem is irrelevant; CON is reserved regardless of extension."""
|
||||
post = _fake_post(tag_categories={"Artist": ["con"]})
|
||||
out = config.render_filename_template("%artist%.%ext%", post, ext=".jpg")
|
||||
assert not out.startswith("con")
|
||||
assert out.startswith("_con")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user