From b2e8bf6e8975030da68e5740948c46e0f91039c9 Mon Sep 17 00:00:00 2001
From: ed <s@ocv.me>
Date: Fri, 17 Jan 2025 06:06:36 +0000
Subject: [PATCH] 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
---
 copyparty/dxml.py   | 51 ++++++++++++++++++++++++++++++++++++++++++---
 copyparty/svchub.py | 10 +++++++++
 copyparty/util.py   |  9 ++++++++
 tests/test_dxml.py  | 25 ++++++++++++++++++++--
 4 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/copyparty/dxml.py b/copyparty/dxml.py
index c44b3afc..b49f060b 100644
--- a/copyparty/dxml.py
+++ b/copyparty/dxml.py
@@ -1,3 +1,6 @@
+# coding: utf-8
+from __future__ import print_function, unicode_literals
+
 import importlib
 import sys
 import xml.etree.ElementTree as ET
@@ -8,6 +11,10 @@ if True:  # pylint: disable=using-constant-test
     from typing import Any, Optional
 
 
+class BadXML(Exception):
+    pass
+
+
 def get_ET() -> ET.XMLParser:
     pn = "xml.etree.ElementTree"
     cn = "_elementtree"
@@ -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)
@@ -49,8 +56,12 @@ class DXMLParser(XMLParser):  # type: ignore
         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:
@@ -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
diff --git a/copyparty/svchub.py b/copyparty/svchub.py
index ae89e7df..01b8b781 100644
--- a/copyparty/svchub.py
+++ b/copyparty/svchub.py
@@ -61,6 +61,7 @@ from .util import (
     alltrace,
     ansi_re,
     build_netmap,
+    expat_ver,
     load_ipu,
     min_ex,
     mp,
@@ -696,6 +697,15 @@ class SvcHub(object):
             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
 
diff --git a/copyparty/util.py b/copyparty/util.py
index 979b6a5f..aafe224b 100644
--- a/copyparty/util.py
+++ b/copyparty/util.py
@@ -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:
diff --git a/tests/test_dxml.py b/tests/test_dxml.py
index 2dc23f6e..309528a5 100644
--- a/tests/test_dxml.py
+++ b/tests/test_dxml.py
@@ -20,7 +20,8 @@ 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">
 ]>
@@ -28,7 +29,8 @@ class TestDXML(unittest.TestCase):
         _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">
 ]>
@@ -40,6 +42,25 @@ class TestDXML(unittest.TestCase):
         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:">