Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upIntroduce xml_safe as a module to control xml opearations #498
Conversation
c00kiemon5ter
force-pushed the
c00kiemon5ter:fix-ensure-xml-safe-operations
branch
2 times, most recently
from
88d3a90
to
1845dd7
|
Would it be worth printing a warning if defusedxml isn't used? |
This is related to CVE-2017-11427[0] and VU#475445[1] Related issues: #496 #497 Reported by duo[2] through this blog post[3] pysaml2 is not affected, as, by default, the xml.etree.ElementTree and xml.etree.cElementTree parsers ignore comments. However, this commit makes sure that the ElementTree being used is set correctly through defusexml lib and centralizes the control of which functions are exposed and available for usage in the code. Any code that needs a function to parse, modify or serialize XML should be obtain through the xml_safe module. The new module asks defusexml to provide the function and if it is not available it will fallback to the one provided by xml.etree.cElementTree. This is a guarantee that functions like parse, fromstring et al are provided by defusexml lib. [0]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2017-11427 [1]: https://www.kb.cert.org/vuls/id/475445 [2]: https://duo.com [3]: https://duo.com/blog/duo-finds-saml-vulnerabilities-affecting-multiple-implementations
c00kiemon5ter
force-pushed the
c00kiemon5ter:fix-ensure-xml-safe-operations
branch
from
1845dd7
to
ab34e7c
|
I wonder, why this PR still not merged? |
|
Caught up into other aspects. This is not ready, it's just a start. Ideally we should not need to rely on |
|
And what alternative can you propose? |
|
Why is defusedxml not an option? It addresses several security concerns present in python's standard parser. I think falling back onto standard xml or another library is a good option, but I still think using defusedxml by default is preferable. |
|
|
This is related to CVE-2017-11427 and VU#475445
Related issues: #496, #497
Reported by duo through this blog post
pysaml2 is not affected, as, by default, the
xml.etree.ElementTreeandxml.etree.cElementTreeparsers ignore comment nodes. However, this commit makes sure that theElementTreebeing used is set correctly throughdefusexmllib and centralizes the control of which functions are exposed and available for usage in the code. Any code that needs a function to parse, modify or serialize XML should be obtained through thexml_safemodule. The new module asksdefusexmlto provide the function and if it is not available it will fallback to the one provided byxml.etree.cElementTree. This is a guarantee that functions likeparse,fromstringet al are provided bydefusexmllib.All Submissions:
I am putting this here mostly to get feedback and I will soon add tests and reformat this PR to match the problem/solution format that the template suggests.