Sanitize player names in IF save paths to prevent path traversal
This commit is contained in:
parent
57afe9a3ce
commit
8893525647
2 changed files with 23 additions and 1 deletions
|
|
@ -1,6 +1,7 @@
|
||||||
"""Interactive fiction session management via dfrotz subprocess."""
|
"""Interactive fiction session management via dfrotz subprocess."""
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import re
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import TYPE_CHECKING
|
from typing import TYPE_CHECKING
|
||||||
|
|
@ -31,7 +32,10 @@ class IFSession:
|
||||||
@property
|
@property
|
||||||
def save_path(self) -> Path:
|
def save_path(self) -> Path:
|
||||||
"""Return path to save file for this player/game combo."""
|
"""Return path to save file for this player/game combo."""
|
||||||
return self._data_dir / "if_saves" / self.player.name / f"{self.game_name}.qzl"
|
# Sanitize player name to prevent path traversal attacks
|
||||||
|
# Account creation doesn't validate names, so defensive check here
|
||||||
|
safe_name = re.sub(r'[^a-zA-Z0-9_-]', '_', self.player.name)
|
||||||
|
return self._data_dir / "if_saves" / safe_name / f"{self.game_name}.qzl"
|
||||||
|
|
||||||
def _ensure_save_dir(self) -> None:
|
def _ensure_save_dir(self) -> None:
|
||||||
"""Create save directory if it doesn't exist."""
|
"""Create save directory if it doesn't exist."""
|
||||||
|
|
|
||||||
|
|
@ -268,6 +268,24 @@ def test_save_path_property(tmp_path):
|
||||||
assert save_path == tmp_path / "if_saves" / "tester" / "zork.qzl"
|
assert save_path == tmp_path / "if_saves" / "tester" / "zork.qzl"
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_path_sanitizes_malicious_names(tmp_path):
|
||||||
|
"""save_path sanitizes player names to prevent path traversal."""
|
||||||
|
player = MagicMock()
|
||||||
|
player.name = "../../etc/passwd"
|
||||||
|
session = IFSession(player, "/path/to/zork.z5", "zork")
|
||||||
|
|
||||||
|
# Override data_dir for testing
|
||||||
|
session._data_dir = tmp_path
|
||||||
|
|
||||||
|
save_path = session.save_path
|
||||||
|
# Should sanitize to replace non-alphanumeric chars with underscores
|
||||||
|
# "../../etc/passwd" becomes "______etc_passwd"
|
||||||
|
assert ".." not in str(save_path)
|
||||||
|
assert save_path == tmp_path / "if_saves" / "______etc_passwd" / "zork.qzl"
|
||||||
|
# Verify it's still within the if_saves directory
|
||||||
|
assert tmp_path / "if_saves" in save_path.parents
|
||||||
|
|
||||||
|
|
||||||
def test_ensure_save_dir_creates_directories(tmp_path):
|
def test_ensure_save_dir_creates_directories(tmp_path):
|
||||||
"""_ensure_save_dir() creates parent directories."""
|
"""_ensure_save_dir() creates parent directories."""
|
||||||
player = MagicMock()
|
player = MagicMock()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue