T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/read_docx.py:195
- Finding
- Unhardened XML Parsing and Unbounded DOCX Archive Processing<![CDATA[ ## Vulnerability Details **File Location**: `scripts/read_docx.py:195-224`; `scripts/apply_edits_docx.py:410-419` **Vulnerability Type**: Unsafe processing of attacker-controlled XML and ZIP archive members **Risk Level**: Medium ### Vulnerable Code `scripts/read_docx.py:195-224`: ```python def _parse_comments(zipf: zipfile.ZipFile) -> list[dict[str, Any]]: try: data = zipf.read("word/comments.xml") except KeyError: return [] root = etree.fromstring(data) out = [] for c in root.findall(f".//{{{W_NS}}}comment"): out.append({ "id": _attr(c, "id"), "author": _attr(c, "author"), "date": _attr(c, "date"), "text": _text(c).strip(), }) return out def read_docx(path: str | Path) -> dict[str, Any]: """Read docx and return standard structure: { body, comments, path }. Body blocks have type, segments, blockIndex.""" path = Path(path) if not path.exists(): return {"error": f"File not found: {path}"} try: with zipfile.ZipFile(path, "r") as z: doc = z.read("word/document.xml") comments = _parse_comments(z) except Exception as e: return {"error": str(e)} root = etree.fromstring(doc) ``` `scripts/apply_edits_docx.py:410-419`: ```python with zipfile.ZipFile(docx_path, "r") as z_in: doc_bytes = z_in.read("word/document.xml") try: comments_bytes = z_in.read("word/comments.xml") comments_root = etree.fromstring(comments_bytes) except KeyError: comments_root = None comments_bytes = None doc_root = etree.fromstring(doc_bytes) ``` ### Technical Analysis DOCX files are ZIP archives containing XML resources. Both scripts treat the DOCX as potentially arbitrary input, load selected archive members entirely into memory with `ZipFile.read()`, and then parse those bytes through `etree.fromstring()` without explicitly ...[truncated 2389 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Create and consistently use an explicitly hardened parser: ```python def secure_xml_parser(): return etree.XMLParser( resolve_entities=False, load_dtd=False, no_network=True, huge_tree=False, recover=False, ) root = etree.fromstring(data, parser=secure_xml_parser()) ``` 2. Reject XML containing a `DOCTYPE` declaration before parsing: ```python if b"<!DOCTYPE" in data.upper(): raise ValueError("DOCTYPE declarations are not permitted") ``` 3. Consider using `defusedxml` where compatible to obtain defensive handling for common XML denial-of-service and entity attacks. 4. Inspect `ZipInfo` metadata before reading members. Enforce conservative limits for: - Maximum individual uncompressed member size - Maximum total uncompressed size - Maximum member count - Maximum compression ratio 5. Read archive members through a bounded streaming routine rather than loading unlimited data with `ZipFile.read()`. 6. Apply document-complexity limits after parsing, including maximum nesting depth, element count, text size, and output size. 7. Process untrusted documents in a sandbox with memory, CPU, execution-time, and filesystem-access restrictions. 8. Add regression tests using oversized archive members, high-compression payloads, deeply nested XML, internal entity expansion, external entity declarations, and malformed XML. ]]>