[Security] Incomplete defusedxml migration — XML parsing still uses unsafe stdlib in 4 files #37

New issue

Closed

opened 2026-06-22 16:23:19 +00:00 by Mark · 0 comments

Mark commented

2026-06-22 16:23:19 +00:00

Owner

Regression of #20

Issue #20 ("[Security] MEDIUM: 17 Python security issue(s)") was closed with
the claim that defusedxml replaced xml.etree.ElementTree. The fix commit
(cd5e9f1) patched src/intent_resolver/dom_resolver.py but missed four
other files that still use the unsafe import.

Bandit re-scan (today, against current `main`)

>> Issue: [B405:blacklist] Using xml.etree.ElementTree
   Location: src/raster/raster_converter.py:9:0
>> Issue: [B405:blacklist] Using xml.etree.ElementTree
   Location: src/svg_annotation/element_classifier.py:10:0
>> Issue: [B405:blacklist] Using xml.etree.ElementTree
   Location: src/svg_annotation/semantic_analyzer.py:9:0
>> Issue: [B405:blacklist] Using xml.etree.ElementTree
   Location: src/svg_annotation/style_processor.py:10:0

All four parse user-supplied SVG / config XML. This is the exact attack
surface that defusedxml exists to mitigate (XXE, billion-laughs, etc.).

Impact

When the HTTP service (per "Add HTTP service layer" issue) starts ingesting
SVG from network callers, every request that hits the SVG annotation
pipeline is exposed to the XML vulnerabilities that defusedxml was
supposed to close.

Fix

Replace import xml.etree.ElementTree as ET with
from defusedxml import ElementTree as ET in all four files. Audit
remaining ET.parse, ET.fromstring, ET.iterparse, ET.XMLParser
calls to make sure they all use the defusedxml-backed versions.

Add a pre-commit bandit check that fails on any new import xml.etree.ElementTree in src/.

Acceptance

bandit -r src/ reports zero B405 / B314 findings.
A pre-commit hook (or CI step) enforces this on new commits.
The closed issue #20 stays closed (work actually complete this time).

## Regression of #20 Issue #20 ("[Security] MEDIUM: 17 Python security issue(s)") was closed with the claim that `defusedxml` replaced `xml.etree.ElementTree`. The fix commit (`cd5e9f1`) patched `src/intent_resolver/dom_resolver.py` but **missed four other files** that still use the unsafe import. ## Bandit re-scan (today, against current `main`) ``` >> Issue: [B405:blacklist] Using xml.etree.ElementTree Location: src/raster/raster_converter.py:9:0 >> Issue: [B405:blacklist] Using xml.etree.ElementTree Location: src/svg_annotation/element_classifier.py:10:0 >> Issue: [B405:blacklist] Using xml.etree.ElementTree Location: src/svg_annotation/semantic_analyzer.py:9:0 >> Issue: [B405:blacklist] Using xml.etree.ElementTree Location: src/svg_annotation/style_processor.py:10:0 ``` All four parse user-supplied SVG / config XML. This is the exact attack surface that `defusedxml` exists to mitigate (XXE, billion-laughs, etc.). ## Impact When the HTTP service (per "Add HTTP service layer" issue) starts ingesting SVG from network callers, every request that hits the SVG annotation pipeline is exposed to the XML vulnerabilities that `defusedxml` was supposed to close. ## Fix Replace `import xml.etree.ElementTree as ET` with `from defusedxml import ElementTree as ET` in all four files. Audit remaining `ET.parse`, `ET.fromstring`, `ET.iterparse`, `ET.XMLParser` calls to make sure they all use the defusedxml-backed versions. Add a pre-commit `bandit` check that fails on any new `import xml.etree.ElementTree` in `src/`. ## Acceptance - `bandit -r src/` reports zero `B405` / `B314` findings. - A pre-commit hook (or CI step) enforces this on new commits. - The closed issue #20 stays closed (work actually complete this time).