anthropics / anthropics/skills

[Security] Use defusedxml instead of xml.etree.ElementTree in evaluation.py

Open
#58 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
176k
Forks
20.9k
Avg merge
7h 21m
Merged PRs (30d)
5

Description

### Summary
`mcp-builder/scripts/evaluation.py` line 13 uses `xml.etree.ElementTree` for parsing evaluation files. While Python 3.13+ blocks external entity expansion by default, `defusedxml` is recommended for defense in depth and protection against XML entity expansion DoS attacks.

**Severity:** Low / Informational

### Current State
```python
import xml.etree.ElementTree as ET
```

### Issue
Python 3.13+ blocks external entities (XXE) by default, preventing file reading and SSRF attacks. However, internal entity expansion remains unprotected, enabling Billion Laughs DoS attacks.

**Verified behavior:**
- External entities: Blocked (`ParseError: undefined entity`)
- Internal entity expansion: Works (tested: 4-level nesting expands to 30,000 characters)

### Proposed Fix
```python
import defusedxml.ElementTree as ET
```

Add to requirements:
```
defusedxml>=0.7.1
```

### Rationale
- Blocks both external entities and internal entity expansion
- Explicit security control independent of Python version defaults
- Minimal code change with comprehensive protection
- Aligns with Python Security and OWASP recommendations

### References
- https://docs.python.org/3/library/xml.html#xml-vulnerabilities
- https://pypi.org/project/defusedxml/
- https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.