From 3744f1eb8a85e5d55e9de8d616845c800fe39273 Mon Sep 17 00:00:00 2001
From: Harald Eilertsen <haraldei@anduin.net>
Date: Sun, 7 Jul 2024 10:36:50 +0200
Subject: Switch to useing XMLReader to parse XML payloads.

XMLParser would expand entities by default, which could make us
susceptible both to XXE attacks, and the billion laughs attack.

By default XMLReader does _not_ expand entities, so it's a safer choice.

This also changes the XmlRpcMethod::parse() function to throw a runtime
exception if the XML payload could not be parsed, and to return null if
the payload does not contain a valid <methodName> element.

In cases where we're unable to parse the payload as a valid XML-RPC
request, we fall back to saving the full request info as before.
---
 tests/unit/XmlRpcTest.php | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'tests')

diff --git a/tests/unit/XmlRpcTest.php b/tests/unit/XmlRpcTest.php
index c245917..25dc186 100644
--- a/tests/unit/XmlRpcTest.php
+++ b/tests/unit/XmlRpcTest.php
@@ -23,4 +23,27 @@ class XmlRpcTest extends TestCase
         $this->assertEquals('wp.getUsersBlogs', $method->name);
         $this->assertEquals(['someuser', 'verysecretpassword'], $method->params);
     }
+
+    public function testShouldNotExpandEntities(): void
+    {
+        $payload = <<<'XML'
+            <!DOCTYPE foo [ <!ENTITY xxx "expanded entity"> ]>
+            <methodCall>
+                <methodName>&xxx;</methodName>
+            </methodCall>
+            XML;
+
+        $method = XmlRpcMethod::parse($payload);
+
+        $this->assertNull($method);
+    }
+
+    public function testInvalidXMLShouldThrowRuntimeException(): void
+    {
+        $payload = '<someTag>some content</otherTag>';
+
+        $this->expectException(\RuntimeException::class);
+
+        XmlRpcMethod::parse($payload);
+    }
 }
-- 
cgit v1.2.3