selftest dxml on startup:

try to decode some malicious xml on startup; if this succeeds,
then force-disable all xml-based features (primarily WebDAV)

this is paranoid future-proofing against unanticipated changes
in future versions of python, specifically if the importlib or
xml.etree.ET behavior changes in a way that somehow reenables
entity expansion, which (still hypothetically) would probably
be caused by failing to unload the `_elementtree` c-module

no past or present python versions are affected by this change
This commit is contained in:
ed 2025-01-17 06:06:36 +00:00
parent 170cbe98c5
commit b2e8bf6e89
4 changed files with 90 additions and 5 deletions

View file

@ -1,3 +1,6 @@
# coding: utf-8
from __future__ import print_function, unicode_literals
import importlib import importlib
import sys import sys
import xml.etree.ElementTree as ET import xml.etree.ElementTree as ET
@ -8,6 +11,10 @@ if True: # pylint: disable=using-constant-test
from typing import Any, Optional from typing import Any, Optional
class BadXML(Exception):
pass
def get_ET() -> ET.XMLParser: def get_ET() -> ET.XMLParser:
pn = "xml.etree.ElementTree" pn = "xml.etree.ElementTree"
cn = "_elementtree" cn = "_elementtree"
@ -34,7 +41,7 @@ def get_ET() -> ET.XMLParser:
XMLParser: ET.XMLParser = get_ET() XMLParser: ET.XMLParser = get_ET()
class DXMLParser(XMLParser): # type: ignore class _DXMLParser(XMLParser): # type: ignore
def __init__(self) -> None: def __init__(self) -> None:
tb = ET.TreeBuilder() tb = ET.TreeBuilder()
super(DXMLParser, self).__init__(target=tb) super(DXMLParser, self).__init__(target=tb)
@ -49,8 +56,12 @@ class DXMLParser(XMLParser): # type: ignore
raise BadXML("{}, {}".format(a, ka)) raise BadXML("{}, {}".format(a, ka))
class BadXML(Exception): class _NG(XMLParser): # type: ignore
pass def __int__(self) -> None:
raise BadXML("dxml selftest failed")
DXMLParser = _DXMLParser
def parse_xml(txt: str) -> ET.Element: def parse_xml(txt: str) -> ET.Element:
@ -59,6 +70,40 @@ def parse_xml(txt: str) -> ET.Element:
return parser.close() # type: ignore return parser.close() # type: ignore
def selftest() -> bool:
qbe = r"""<!DOCTYPE d [
<!ENTITY a "nice_bakuretsu">
]>
<root>&a;&a;&a;</root>"""
emb = r"""<!DOCTYPE d [
<!ENTITY a SYSTEM "file:///etc/hostname">
]>
<root>&a;</root>"""
# future-proofing; there's never been any known vulns
# regarding DTDs and ET.XMLParser, but might as well
# block them since webdav-clients don't use them
dtd = r"""<!DOCTYPE d SYSTEM "a.dtd">
<root>a</root>"""
for txt in (qbe, emb, dtd):
try:
parse_xml(txt)
t = "WARNING: dxml selftest failed:\n%s\n"
print(t % (txt,), file=sys.stderr)
return False
except BadXML:
pass
return True
DXML_OK = selftest()
if not DXML_OK:
DXMLParser = _NG
def mktnod(name: str, text: str) -> ET.Element: def mktnod(name: str, text: str) -> ET.Element:
el = ET.Element(name) el = ET.Element(name)
el.text = text el.text = text

View file

@ -61,6 +61,7 @@ from .util import (
alltrace, alltrace,
ansi_re, ansi_re,
build_netmap, build_netmap,
expat_ver,
load_ipu, load_ipu,
min_ex, min_ex,
mp, mp,
@ -696,6 +697,15 @@ class SvcHub(object):
if self.args.bauth_last: if self.args.bauth_last:
self.log("root", "WARNING: ignoring --bauth-last due to --no-bauth", 3) self.log("root", "WARNING: ignoring --bauth-last due to --no-bauth", 3)
if not self.args.no_dav:
from .dxml import DXML_OK
if not DXML_OK:
if not self.args.no_dav:
self.args.no_dav = True
t = "WARNING:\nDisabling WebDAV support because dxml selftest failed. Please report this bug;\n%s\n...and include the following information in the bug-report:\n%s | expat %s\n"
self.log("root", t % (URL_BUG, VERSIONS, expat_ver()), 1)
def _process_config(self) -> bool: def _process_config(self) -> bool:
al = self.args al = self.args

View file

@ -495,6 +495,15 @@ def py_desc() -> str:
) )
def expat_ver() -> str:
try:
import pyexpat
return ".".join([str(x) for x in pyexpat.version_info])
except:
return "?"
def _sqlite_ver() -> str: def _sqlite_ver() -> str:
assert sqlite3 # type: ignore # !rm assert sqlite3 # type: ignore # !rm
try: try:

View file

@ -20,7 +20,8 @@ def _parse(txt):
class TestDXML(unittest.TestCase): class TestDXML(unittest.TestCase):
def test1(self): def test_qbe(self):
# allowed by default; verify that we stopped it
txt = r"""<!DOCTYPE qbe [ txt = r"""<!DOCTYPE qbe [
<!ENTITY a "nice_bakuretsu"> <!ENTITY a "nice_bakuretsu">
]> ]>
@ -28,7 +29,8 @@ class TestDXML(unittest.TestCase):
_parse(txt) _parse(txt)
ET.fromstring(txt) ET.fromstring(txt)
def test2(self): def test_ent_file(self):
# NOT allowed by default; should still be blocked
txt = r"""<!DOCTYPE ext [ txt = r"""<!DOCTYPE ext [
<!ENTITY ee SYSTEM "file:///bin/bash"> <!ENTITY ee SYSTEM "file:///bin/bash">
]> ]>
@ -40,6 +42,25 @@ class TestDXML(unittest.TestCase):
except ET.ParseError: except ET.ParseError:
pass pass
def test_ent_ext(self):
# NOT allowed by default; should still be blocked
txt = r"""<!DOCTYPE ext [
<!ENTITY ee SYSTEM "http://example.com/a.xml">
]>
<root>&ee;</root>"""
_parse(txt)
def test_dtd(self):
# allowed by default; verify that we stopped it
txt = r"""<!DOCTYPE d SYSTEM "a.dtd">
<root>a</root>"""
_parse(txt)
ET.fromstring(txt)
##
## end of negative/security tests; the rest is functional
##
def test3(self): def test3(self):
txt = r"""<?xml version="1.0" ?> txt = r"""<?xml version="1.0" ?>
<propfind xmlns="DAV:"> <propfind xmlns="DAV:">