T09 · Insecure Skill Coding Practices
Warning
- Location
- save_dream.py:13
- Finding
- Existing Dream Records Can Be Silently Overwritten## 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**: Medium **Vulnerable 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.md` and `YYYY-MM-DD-003.md` exist, the function counts two files and selects `YYYY-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:** 1. Arrange for a numbering gap in the current day's records, such as by deleting or moving `YYYY-MM-DD-002.md` while `001.md` and `003.md` remain. 2. Invoke `save_dream.py` with a new dream record. 3. `next_filename()` observes two matching files and calculates index `3`. 4. The script selects the existing `YYYY-MM-DD-003.md` path. 5. `write_text()` truncates and replaces the existing journal record without warning. **Concurrent-write path:** 1. Invoke two instances of `save_dream.py` concurrently. 2. Both instances count the same existing files before either creates its destination. 3. Both select the same next filename. 4. Both write non-exclusively ...[truncated 579 chars]
- Remediation
- ## Remediation Suggestions 1. Parse valid numeric suffixes and choose a number greater than the highest existing suffix rather than using the number of matching files. 2. Create the destination atomically with exclusive mode, such as `open("x", encoding="utf-8")`, so an existing record can never be silently truncated. 3. If exclusive creation reports `FileExistsError`, recompute the sequence and retry. This addresses concurrent invocations. 4. Validate filenames against the exact expected pattern before considering them during sequence allocation. 5. 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. 6. 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 ```