Importing a career file: the DOCTYPE refusal reads only the first 4 KB as ASCII, so a later or UTF-16 DTD is parsed #378

New issue

Closed

opened 2026-10-01 10:46:15 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:15 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the importer kind's refusals and the Europass XML reader, and settled by feeding crafted documents to reader.read.

Related to #105.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/europass/reader.py:281-293 (read_xml, with fromstring at line 293) and its callers of the refusal at :156 and :240; plugins/base.py:363-364 and resume/views.py:442 have not moved. The same three documents give the same three results.

refuse_unreadable is the only thing standing between an uploaded career file and the standard library's XML parser, and it looks for the bytes <!DOCTYPE only in the first 4096 bytes and only in an ASCII-compatible encoding. XML allows any amount of comment or processing instruction before the DOCTYPE, and allows UTF-16. Either way the declaration is not seen, the file is accepted, and read_xml parses it with xml.etree.ElementTree.fromstring, which expands internal entities. The guarantee the code, its comment and the wiki all state ("refused outright, without being parsed") does not hold.

Where: plugins/base.py:363-364 (refuse_unreadable: head = data[:4096]... and re.search(rb"<!DOCTYPE", data[:4096], re.I)); plugins/europass/reader.py:404-416 (read_xml, whose docstring says the refusal "is what makes fromstring below safe to call", and ElementTree.fromstring(data) # noqa: S314 - no DOCTYPE); the callers resume/views.py:442, plugins/europass/reader.py:241 and :325 (the XML attached to a Europass PDF). The wiki page Your-career-record.md:285-287 promises the refusal.
What happens: checked against the snapshot, with a Europass SkillsPassport document that declares an internal entity and uses it in FirstName:
- the document as it is: refused with ImportRefused, as intended;
- the same document with a comment of 5000 spaces before the DOCTYPE: read, and person["first_name"] comes back as the entity's expansion;
- the same document encoded as UTF-16LE without a byte-order mark: read and expanded as well, with or without the padding (_read_document sends it to read_xml because its first byte is <).
Who can trigger it: any signed-in account holder, through Import a Europass CV, with an XML file or a PDF carrying one attached.
Why it matters: entity expansion is the class of attack the refusal exists to keep away from the parser. With the standard library parser nothing is fetched, so what remains is expansion bounded only by expat's own amplification limit (expat 2.8.2 in the snapshot's environment), which still lets a 5 MB file expand far beyond its size in a web worker's memory. plugins/api.py exports refuse_unreadable to third-party importers as the kind's guarantee; Writing-a-plugin.md:1212-1218 does tell their authors to use a parser that resolves no entities, but the core's own reader does not follow that advice. defusedxml is already a dependency and core/pictures.py:41 parses SVG with it, so the Europass reader is the one XML parser in the core that does not.
Fix: parse in read_xml with defusedxml.ElementTree.fromstring (its defaults forbid a DTD's entities and external references; pass forbid_dtd=True to refuse any DOCTYPE outright) and map defusedxml's exceptions to the existing refusal sentence. Make refuse_unreadable decide with a parser rather than a byte search: feed XML-looking data to an expat parser whose StartDoctypeDeclHandler raises, or detect the encoding first, so neither position nor encoding matters. Drop the "safe because of the refusal" wording in the docstring, and keep the wiki sentence only once it is true.

Done when

tests/test_importers.py refuses a document whose DOCTYPE follows a comment longer than 4 KB, and the same document in UTF-16LE and UTF-16BE, with and without a byte-order mark.
tests/test_europass.py refuses each of those inside a PDF attachment as well, and read_xml called directly on such a document raises rather than expanding the entity.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the importer kind's refusals and the Europass XML reader, and settled by feeding crafted documents to `reader.read`. Related to #105. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/europass/reader.py:281-293` (`read_xml`, with `fromstring` at line 293) and its callers of the refusal at `:156` and `:240`; `plugins/base.py:363-364` and `resume/views.py:442` have not moved. The same three documents give the same three results. `refuse_unreadable` is the only thing standing between an uploaded career file and the standard library's XML parser, and it looks for the bytes `<!DOCTYPE` only in the first 4096 bytes and only in an ASCII-compatible encoding. XML allows any amount of comment or processing instruction before the DOCTYPE, and allows UTF-16. Either way the declaration is not seen, the file is accepted, and `read_xml` parses it with `xml.etree.ElementTree.fromstring`, which expands internal entities. The guarantee the code, its comment and the wiki all state ("refused outright, without being parsed") does not hold. - **Where:** `plugins/base.py:363-364` (`refuse_unreadable`: `head = data[:4096]...` and `re.search(rb"<!DOCTYPE", data[:4096], re.I)`); `plugins/europass/reader.py:404-416` (`read_xml`, whose docstring says the refusal "is what makes `fromstring` below safe to call", and `ElementTree.fromstring(data) # noqa: S314 - no DOCTYPE`); the callers `resume/views.py:442`, `plugins/europass/reader.py:241` and `:325` (the XML attached to a Europass PDF). The wiki page `Your-career-record.md:285-287` promises the refusal. - **What happens:** checked against the snapshot, with a Europass `SkillsPassport` document that declares an internal entity and uses it in `FirstName`: - the document as it is: refused with `ImportRefused`, as intended; - the same document with a comment of 5000 spaces before the DOCTYPE: read, and `person["first_name"]` comes back as the entity's expansion; - the same document encoded as UTF-16LE without a byte-order mark: read and expanded as well, with or without the padding (`_read_document` sends it to `read_xml` because its first byte is `<`). - **Who can trigger it:** any signed-in account holder, through *Import a Europass CV*, with an XML file or a PDF carrying one attached. - **Why it matters:** entity expansion is the class of attack the refusal exists to keep away from the parser. With the standard library parser nothing is fetched, so what remains is expansion bounded only by expat's own amplification limit (expat 2.8.2 in the snapshot's environment), which still lets a 5 MB file expand far beyond its size in a web worker's memory. `plugins/api.py` exports `refuse_unreadable` to third-party importers as the kind's guarantee; `Writing-a-plugin.md:1212-1218` does tell their authors to use a parser that resolves no entities, but the core's own reader does not follow that advice. `defusedxml` is already a dependency and `core/pictures.py:41` parses SVG with it, so the Europass reader is the one XML parser in the core that does not. - **Fix:** parse in `read_xml` with `defusedxml.ElementTree.fromstring` (its defaults forbid a DTD's entities and external references; pass `forbid_dtd=True` to refuse any DOCTYPE outright) and map `defusedxml`'s exceptions to the existing refusal sentence. Make `refuse_unreadable` decide with a parser rather than a byte search: feed XML-looking data to an expat parser whose `StartDoctypeDeclHandler` raises, or detect the encoding first, so neither position nor encoding matters. Drop the "safe because of the refusal" wording in the docstring, and keep the wiki sentence only once it is true. ## Done when - `tests/test_importers.py` refuses a document whose DOCTYPE follows a comment longer than 4 KB, and the same document in UTF-16LE and UTF-16BE, with and without a byte-order mark. - `tests/test_europass.py` refuses each of those inside a PDF attachment as well, and `read_xml` called directly on such a document raises rather than expanding the entity.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:15 +00:00

tiagoagueda added the

security

tier

bug

labels

2026-10-01 10:46:15 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:56 +00:00

Refuse a DOCTYPE by parsing for it, not by searching the first 4 KB

tiagoagueda closed this issue