search

martinblech / xmltodict

Python module that makes working with XML feel like you are working with JSON
MIT License
5.52k stars 462 forks source link

XXE Injection #174

Open An7i21 opened 6 years ago

An7i21 commented 6 years ago

xml.parsers library is known to be vulnerable to XXE based attacks (CVE-2017-9233 for EX) Therefore, the developer of this library decided to use the defusedexpat.pyexpat wrapper which is the secure fork of original Expat library.

defusedexpat protects the XML packages of Python's standard library from several denial of service vulnerabilities and external entity exploits (From the library documentation).

Unfortunately, the implementation of this protection mechanism seems to be relatively permissive: As it can be seen at the first lines of the xmltodict.py the library tries to import the secure library "defusedexpat.pyexpat" but in case when an error occurs (say for example the library isn't installed) the vulnerable "xml.parsers.expat" is loaded, leaving the code vulnerable to an XXE attack.

try: from defusedexpat import pyexpat as expat except ImportError: from xml.parsers import expat

Brightcells commented 6 years ago 
def parse(xml_input, encoding=None, expat=expat, process_namespaces=False,
          namespace_separator=':', disable_entities=True, **kwargs):
rgmz commented 3 years ago

Apparently defusedexpat doesn't support Python versions after 3.4. Attempting to install it results in an error:

Collecting defusedexpat
  Downloading defusedexpat-0.4.zip (275 kB)
     |████████████████████████████████| 275 kB 1.9 MB/s
Using legacy 'setup.py install' for defusedexpat, since package 'wheel' is not installed.
Installing collected packages: defusedexpat
    Running setup.py install for defusedexpat ... error
    ERROR: Command errored out with exit status 1:
    ...
    clang: error: no such file or directory: 'Modules38/pyexpat.c'
    clang: error: no input files
    error: command 'clang' failed with exit status 1
    ----------------------------------------
def parse(xml_input, encoding=None, expat=expat, process_namespaces=False,
          namespace_separator=':', disable_entities=True, **kwargs):
* disable_entities

@Brightcells / @martinblech: Apologies for the necro-bump, but I wanted to confirm if the default value of disable_entities is sufficient to prevent XXE injection.