T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/track_changes.py:297
- Finding
- Unbounded DOCX Archive Extraction and XML Parsing## Vulnerability Details **File Location**: `scripts/track_changes.py`, lines 297–305 **Vulnerability Type**: Uncontrolled resource consumption through untrusted archive extraction and XML parsing **Risk Level**: Medium ### Vulnerable Code ```python def __init__(self, docx_path): self.docx_path = docx_path self.temp_dir = tempfile.mkdtemp(prefix='word_track_changes_') with zipfile.ZipFile(docx_path, 'r') as z: z.extractall(self.temp_dir) self.tree = ET.parse(os.path.join(self.temp_dir, 'word/document.xml')) self.root = self.tree.getroot() ``` ### Technical Analysis The processor treats an input DOCX file as a trusted ZIP archive and extracts every member with `ZipFile.extractall()` before parsing `word/document.xml`. It does not enforce limits on: - The number of archive members - The total uncompressed size - The uncompressed size of an individual member - Compression ratios - The size or structural complexity of the XML input A maliciously constructed DOCX can therefore contain a ZIP bomb or an abnormally large `word/document.xml`. Extraction may consume all available temporary storage, while XML parsing may consume excessive memory or CPU. The temporary directory is created before archive extraction, but initialization and helper functions do not guarantee cleanup through a context manager or `try/finally`. If extraction, parsing, modification, or saving raises an exception, extracted document data may remain in the temporary directory. ### Attack Path 1. An attacker creates a DOCX file containing highly compressed data, an excessive number of ZIP members, or an oversized `word/document.xml`. 2. The attacker supplies the file for processing through any documented CLI or the `TrackChangesProcessor` API. 3. The constructor calls `z.extractall(self.temp_dir)` without validating archive metadata or enforcing extraction quotas. 4. The archive expands until temporary storage is exhausted, or the oversized XML document ...[truncated 827 chars]
- Remediation
- ## Remediation Suggestions 1. Inspect every `ZipInfo` entry before extraction and reject archives that exceed defined limits for: - Member count - Total declared uncompressed size - Per-member uncompressed size - Compression ratio - Maximum permitted XML size 2. Extract only the OOXML parts required by the application instead of calling `extractall()` on the entire archive. 3. Normalize and validate member paths before extraction. Reject absolute paths, parent-directory components, and any destination that resolves outside the temporary directory. 4. Stream archive members while tracking the number of bytes actually written. Do not rely exclusively on attacker-controlled ZIP metadata. 5. Apply explicit XML input-size limits before calling `ET.parse()`. Where practical, use bounded or incremental parsing and reject documents with excessive structural complexity. 6. Guarantee temporary-file cleanup with `tempfile.TemporaryDirectory`, a context manager, or `try/finally`. Cleanup must occur after constructor, parsing, processing, or saving failures. 7. Return a controlled validation error when archive limits are exceeded, and document the supported maximum DOCX size.