T09 · Insecure Skill Coding Practices
- Location
save_dream.py:13- Finding
Existing Dream Records Can Be Silently Overwritten
- Content
View full analysis
Vulnerability Details
File Location:
save_dream.py, lines 13-16 and line 38
Vulnerability Type: Insecure filename allocation and non-exclusive file creation
Risk Level: MediumVulnerable code:
python def next_filename(date_str: str) -> Path: existing = sorted(DREAMS_DIR.glob(f"{date_str}-*.md")) idx = len(existing) + 1 return DREAMS_DIR / f"{date_str}-{idx:03d}.md"python filepath.write_text(content, encoding="utf-8")Technical Analysis
The sequence number is calculated from the number of matching files rather than the highest sequence number already in use. If files
YYYY-MM-DD-001.mdandYYYY-MM-DD-003.mdexist, the function counts two files and selectsYYYY-MM-DD-003.md, which is already occupied.Path.write_text()opens the selected path without exclusive-create semantics and truncates an existing file. Consequently, the collision silently replaces the prior dream record.A separate time-of-check/time-of-use race also exists. Two concurrent processes can inspect the same directory state, derive the same sequence number, and write to the same destination. One write may replace the other, or the resulting content may depend on execution timing.
Attack Path
Numbering-gap path:
- Arrange for a numbering gap in the current day's records, such as by deleting or moving
YYYY-MM-DD-002.mdwhile001.mdand003.mdremain. - Invoke
save_dream.pywith a new dream record. next_filename()observes two matching files and calculates index3.- The script selects the existing
YYYY-MM-DD-003.mdpath. write_text()truncates and replaces the existing journal record without warning.
Concurrent-write path:
- Invoke two instances of
save_dream.pyconcurrently. - Both instances count the same existing files before either creates its destination.
- Both select the same next filename.
- Both write non-exclusively ...[truncated 579 chars]
- Arrange for a numbering gap in the current day's records, such as by deleting or moving
- Remediation
View remediation
Remediation Suggestions
- Parse valid numeric suffixes and choose a number greater than the highest existing suffix rather than using the number of matching files.
- Create the destination atomically with exclusive mode, such as
open("x", encoding="utf-8"), so an existing record can never be silently truncated. - If exclusive creation reports
FileExistsError, recompute the sequence and retry. This addresses concurrent invocations. - Validate filenames against the exact expected pattern before considering them during sequence allocation.
- Consider writing and flushing content to a securely created temporary file before atomically renaming it to the exclusively reserved destination, if crash-consistent writes are required.
- Add tests covering numbering gaps, malformed filenames, pre-existing destination files, and simultaneous save operations.
Example allocation strategy:
python def save_with_retry(date_str: str, content: str) -> Path: indices = [] for path in DREAMS_DIR.glob(f"{date_str}-*.md"): suffix = path.stem.removeprefix(f"{date_str}-") if suffix.isdigit(): indices.append(int(suffix)) idx = max(indices, default=0) + 1 while True: filepath = DREAMS_DIR / f"{date_str}-{idx:03d}.md" try: with filepath.open("x", encoding="utf-8") as output: output.write(content) return filepath except FileExistsError: idx += 1