Skip to content

Commit

Permalink
selftest dxml on startup:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
9001 committed Jan 17, 2025
1 parent 170cbe9 commit b2e8bf6
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 5 deletions.
51 changes: 48 additions & 3 deletions copyparty/dxml.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# coding: utf-8
from __future__ import print_function, unicode_literals

import importlib
import sys
import xml.etree.ElementTree as ET
Expand All @@ -8,6 +11,10 @@
from typing import Any, Optional


class BadXML(Exception):
pass


def get_ET() -> ET.XMLParser:
pn = "xml.etree.ElementTree"
cn = "_elementtree"
Expand All @@ -34,7 +41,7 @@ def get_ET() -> ET.XMLParser:
XMLParser: ET.XMLParser = get_ET()


class DXMLParser(XMLParser): # type: ignore
class _DXMLParser(XMLParser): # type: ignore
def __init__(self) -> None:
tb = ET.TreeBuilder()
super(DXMLParser, self).__init__(target=tb)
Expand All @@ -49,8 +56,12 @@ def nope(self, *a: Any, **ka: Any) -> None:
raise BadXML("{}, {}".format(a, ka))


class BadXML(Exception):
pass
class _NG(XMLParser): # type: ignore
def __int__(self) -> None:
raise BadXML("dxml selftest failed")


DXMLParser = _DXMLParser


def parse_xml(txt: str) -> ET.Element:
Expand All @@ -59,6 +70,40 @@ def parse_xml(txt: str) -> ET.Element:
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:
el = ET.Element(name)
el.text = text
Expand Down
10 changes: 10 additions & 0 deletions copyparty/svchub.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
alltrace,
ansi_re,
build_netmap,
expat_ver,
load_ipu,
min_ex,
mp,
Expand Down Expand Up @@ -696,6 +697,15 @@ def _check_env(self) -> None:
if self.args.bauth_last:
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:
al = self.args

Expand Down
9 changes: 9 additions & 0 deletions copyparty/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
assert sqlite3 # type: ignore # !rm
try:
Expand Down
25 changes: 23 additions & 2 deletions tests/test_dxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ def _parse(txt):


class TestDXML(unittest.TestCase):
def test1(self):
def test_qbe(self):
# allowed by default; verify that we stopped it
txt = r"""<!DOCTYPE qbe [
<!ENTITY a "nice_bakuretsu">
]>
<l>&a;&a;&a;&a;&a;&a;&a;&a;&a;</l>"""
_parse(txt)
ET.fromstring(txt)

def test2(self):
def test_ent_file(self):
# NOT allowed by default; should still be blocked
txt = r"""<!DOCTYPE ext [
<!ENTITY ee SYSTEM "file:///bin/bash">
]>
Expand All @@ -40,6 +42,25 @@ def test2(self):
except ET.ParseError:
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):
txt = r"""<?xml version="1.0" ?>
<propfind xmlns="DAV:">
Expand Down

0 comments on commit b2e8bf6

Please sign in to comment.