classification
Title: Document XML Vulnerabilties
Type: security Stage: resolved
Components: Documentation Versions: Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: benjamin.peterson, christian.heimes, docs@python, dstufft, eric.araujo, georg.brandl, hynek, python-dev
Priority: normal Keywords: patch

Created on 2013-03-25 02:41 by dstufft, last changed 2013-12-22 00:58 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
xmldocs.diff dstufft, 2013-03-25 02:41 Add Warnings to XML docs review
xmldocs2.diff christian.heimes, 2013-03-25 18:31
xmldocs3.diff christian.heimes, 2013-03-26 15:26
Messages (17)
msg185176 - (view) Author: Donald Stufft (dstufft) * (Python committer) Date: 2013-03-25 02:41
Here's a documentation patch (Made against the 2.7 branch) that adds warning to the various xml modules to warn about the insecurity and points towards defusedxml/defusedexpat.
msg185196 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2013-03-25 11:54
I feel like there should be a warning in Doc/library/xml.rst too.

Is there any actual reason why we don’t ship defusedxml with Python and add an easy way to monkeypatch so there’s as little passive barriers as possible to use XML “safely”?

I’m sorry I didn’t speak up on when this was discussed on the ML but I found the discussion…depressing.
msg185199 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-03-25 12:16
Donald: Thanks! I'm going to look at your patch later today.

Hynek: Because the preferred way is another: use patched expat and pyexpat C modules of defusedexpat. It's a fix on C level and still allows a sane amount of entity expansions. defusedxml disallows any XML document that smells even a tiny bit. This approach needs a) more reviews and b) an API to enable the limitations-
msg185222 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-03-25 18:31
Updated patch with more infos and also a link to defusedexpat.
msg185224 - (view) Author: Donald Stufft (dstufft) * (Python committer) Date: 2013-03-25 18:45
Update looks fine to me, I'm not the best at docs I just wanted to get at least a jumping off point.
msg185258 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-03-26 05:19
LGTM.
msg185263 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-03-26 09:21
Benjaman and Georg, what do the RMs feel about the doc patch?
msg185265 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-03-26 09:45
* I would take out the "erroneous" of "erroneous or maliciously constructed" in the disclaimers.  The odds of creating one of the "bombings" by chance are slim.

* The names of attacks in the table are quite opaque if you haven't heard of them.  They should be linked/explained.  (Also, the csv-table construction looks quite strange; a normal reST table would be preferred.)

* I don't think the warning for SAX needs to be repeated three times.

* Not sure the reader will get the reason for having both "defusedxml" and "defusedexpat".
msg185266 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-03-26 10:09
1. sounds fine to me

2. I can copy some text from the README.txt of defusedxml. CSV table was easier to maintain for me. What's a good tool to create and modify sphinx tables?

3. No strong opinion here, better safe than sorry?

4. IMO the warning should mention both. Perhaps a better explanation will help?
msg185267 - (view) Author: Donald Stufft (dstufft) * (Python committer) Date: 2013-03-26 11:36
FWIW I put the warning on all the sax pages just because there's no way to know which page a user will go to if they are coming in via google.
msg185268 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-03-26 11:45
2. That would be good.  For this table the "simple" rst tables should be fine:

=====  =====
head   head
=====  =====
body   body
body   body
=====  =====

3. Once per XML logical handling module/package seems quite enough.

4. The warnings only link to the section about "XML Vulnerabilities", which is good.  But that section should not only mention that the defusedxml description explains everything, but also *what the hell* it contains :)
msg185275 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-03-26 15:26
Here we go again!
msg185277 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-03-26 15:30
I still see "erroneous" in there... otherwise looks good to me.
msg185280 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-26 16:53
New changeset e7795a178b0a by Christian Heimes in branch '3.2':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/e7795a178b0a

New changeset 65e8ac5f073f by Christian Heimes in branch '3.3':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/65e8ac5f073f

New changeset 1ebefb6ae9a9 by Christian Heimes in branch 'default':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/1ebefb6ae9a9

New changeset e87364449954 by Christian Heimes in branch '2.7':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/e87364449954
msg185562 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-30 14:37
New changeset 91bb6d7ae833 by Christian Heimes in branch '2.7':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/91bb6d7ae833
msg186128 - (view) Author: Roundup Robot (python-dev) Date: 2013-04-06 14:43
New changeset f45902f8c7d7 by Christian Heimes in branch '3.2':
Issue 17538: Document XML vulnerabilties
http://hg.python.org/cpython/rev/f45902f8c7d7
msg186133 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-06 15:21
Christian: there are people strongly disagreeing with the description of minidom as “lightweight”, could you edit the libary/xml.rst file you added to say “minimal” instead?  See c2ae1ed03853 and #11379 if you want more info.
History
Date User Action Args
2013-12-22 00:58:10pitrousetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2013-04-06 15:21:54eric.araujosetmessages: + msg186133
2013-04-06 14:43:57python-devsetmessages: + msg186128
2013-03-30 14:37:33python-devsetmessages: + msg185562
2013-03-26 16:53:33python-devsetnosy: + python-dev
messages: + msg185280
2013-03-26 15:30:43georg.brandlsetmessages: + msg185277
2013-03-26 15:26:23christian.heimessetfiles: + xmldocs3.diff

messages: + msg185275
stage: patch review
2013-03-26 11:45:08georg.brandlsetmessages: + msg185268
2013-03-26 11:36:35dstufftsetmessages: + msg185267
2013-03-26 10:09:42christian.heimessetmessages: + msg185266
2013-03-26 09:45:29georg.brandlsetmessages: + msg185265
2013-03-26 09:21:08christian.heimessetnosy: + georg.brandl, benjamin.peterson
messages: + msg185263
2013-03-26 05:19:12eric.araujosetnosy: + eric.araujo

messages: + msg185258
versions: + Python 3.2
2013-03-25 18:45:41dstufftsetmessages: + msg185224
2013-03-25 18:31:30christian.heimessetfiles: + xmldocs2.diff

messages: + msg185222
2013-03-25 12:16:49christian.heimessetmessages: + msg185199
2013-03-25 11:54:33hyneksetassignee: docs@python
type: security
components: + Documentation, - Library (Lib), XML
versions: + Python 3.3, Python 3.4
nosy: + hynek, docs@python

messages: + msg185196
2013-03-25 02:41:26dstufftcreate