T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/translator.py:133
- Finding
- XML External Entity Resolution in Untrusted XHTML and EPUB Documents## Vulnerability Details **File Location**: `scripts/translator.py:133-134` and `scripts/translator.py:176-177` **Duplicate Location**: `classical-chinese-translator/scripts/translator.py:133-134` and `classical-chinese-translator/scripts/translator.py:176-177` **Vulnerability Type**: XML External Entity exposure through insufficiently hardened XML parsing **Risk Level**: High **Vulnerable code in XHTML processing:** ```python # Parse XML safely parser = etree.XMLParser(recover=True, encoding='utf-8') root = etree.fromstring(content.encode('utf-8'), parser) ``` **Vulnerable code in EPUB document processing:** ```python # Parse and translate XHTML content content = item.get_content().decode('utf-8') parser = etree.XMLParser(recover=True, encoding='utf-8') root = etree.fromstring(content.encode('utf-8'), parser) ``` ### Technical Analysis The Skill parses attacker-controlled XHTML and EPUB document content with `lxml.etree.XMLParser` without explicitly disabling DTD loading and external entity resolution. The parser is created without security controls such as `resolve_entities=False`, `load_dtd=False`, and `no_network=True`. In affected lxml/libxml2 configurations, a malicious document can include a `DOCTYPE` declaration defining an external entity that points to a locally readable file. If the entity is resolved during parsing, its contents become part of the parsed XML tree. The application subsequently processes paragraph content through `itertext()`: ```python original_text = ''.join(p_elem.itertext()) ``` It then places that text into a newly generated paragraph and serializes the document to the selected output file. Consequently, resolved local-file content can be persisted in the translated output. The use of `recover=True` further weakens strict input validation because malformed XML is repaired where possible instead of being rejected. This can make it more difficult to enforce assumptions a ...[truncated 1867 chars]
- Remediation
- ## Remediation Suggestions 1. Create a hardened parser that explicitly disables external entities, DTD loading, and network access: ```python parser = etree.XMLParser( encoding='utf-8', resolve_entities=False, load_dtd=False, no_network=True, recover=False ) ``` 2. Apply the hardened parser consistently to direct XHTML processing and every document item extracted from an EPUB. 3. Reject documents containing `DOCTYPE` or entity declarations before parsing when these features are not required: ```python if b'<!DOCTYPE' in raw_content.upper() or b'<!ENTITY' in raw_content.upper(): raise ValueError("DTD and entity declarations are not permitted") ``` 4. Use strict parsing with `recover=False`. Report malformed input to the user instead of silently repairing and processing it. 5. Consider using `defusedxml` or an equivalent hardened XML-processing layer where compatible with the document workflow. 6. Add regression tests containing: - An external entity referencing a local file. - Parameter entities and nested entity declarations. - Network-based external entities. - Malformed XML that previously succeeded through recovery mode. - Malicious XHTML embedded inside an EPUB. 7. Ensure tests verify that no referenced content appears in the parsed tree or generated output and that prohibited declarations cause processing to fail safely.