T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/newsletter.py:254
- Finding
- Arbitrary JSON File Modification Through Unsanitized Draft Identifier<![CDATA[ ## Vulnerability Details **File Location**: `scripts/newsletter.py:254-279` and `scripts/newsletter.py:287-310` **Vulnerability Type**: Path traversal and arbitrary file modification **Risk Level**: High ### Vulnerable Code ```python def schedule(self, draft_id: str, send_time: str, timezone: Optional[str] = None, optimal: bool = False) -> bool: """Schedule newsletter for delivery.""" draft_file = self.drafts_dir / f"{draft_id}.json" if not draft_file.exists(): print(f"✗ Draft not found: {draft_id}") return False # Load draft newsletter_data = json.loads(draft_file.read_text()) # Parse send time if optimal: # Calculate optimal send time (9 AM in recipient timezone) send_dt = datetime.now().replace(hour=9, minute=0, second=0, microsecond=0) if send_dt < datetime.now(): send_dt += timedelta(days=1) else: send_dt = datetime.fromisoformat(send_time.replace('Z', '+00:00')) # Update draft newsletter_data['scheduled_at'] = send_dt.timestamp() newsletter_data['status'] = 'scheduled' draft_file.write_text(json.dumps(newsletter_data, indent=2)) ``` The same flaw is present in the sending operation: ```python def send(self, draft_id: str, test_email: Optional[str] = None, segment: Optional[str] = None, platform: Optional[str] = None) -> bool: """Send newsletter.""" draft_file = self.drafts_dir / f"{draft_id}.json" if not draft_file.exists(): print(f"✗ Draft not found: {draft_id}") return False newsletter_data = json.loads(draft_file.read_text()) platform = platform or newsletter_data.get('platform', 'substack') print(f"✓ Sending: {draft_id}") print(f" Platform: {platform}") if test_email: print(f" Test mode: sending to {test_email}") # In real implementation, would send test email else: print(f" Recipients: All subscribers") # In real ...[truncated 2245 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate draft identifiers against the exact format produced by `_generate_id()`: ```python import re if not re.fullmatch(r"draft-[0-9]+-[0-9a-f]{8}", draft_id): raise ValueError("Invalid draft ID") ``` 2. Resolve and verify the final path before accessing it: ```python drafts_root = self.drafts_dir.resolve() draft_file = (drafts_root / f"{draft_id}.json").resolve() if draft_file.parent != drafts_root: raise ValueError("Draft path escapes the drafts directory") ``` 3. Explicitly reject absolute paths, path separators, `.` components, and `..` components. 4. Centralize safe draft lookup in one helper and use it in `schedule()`, `send()`, and any future draft operations. 5. Write updates atomically to a temporary file in the same protected directory and then replace the intended draft. 6. Add tests covering relative traversal, absolute paths, encoded separators, malformed identifiers, and symlink-related boundary cases. ]]>