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.

1 files changed, 23 insertions, 0 deletions

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);
+    }
 }