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.
---
 src/XmlRpcMethod.php    | 63 +++++++++++++++++++++++++++++++++----------------
 src/process-request.php | 18 ++++++++------
 2 files changed, 54 insertions(+), 27 deletions(-)

(limited to 'src')

diff --git a/src/XmlRpcMethod.php b/src/XmlRpcMethod.php
index 2167e53..001169b 100644
--- a/src/XmlRpcMethod.php
+++ b/src/XmlRpcMethod.php
@@ -7,6 +7,8 @@
 
 namespace VolseNet\Webtrap;
 
+use XMLReader;
+
 /**
  * A representation of an XML-RPC method call.
  *
@@ -24,34 +26,41 @@ class XmlRpcMethod
      *
      * @param string $payload   The raw XML representation of the method call.
      *
-     * @return XmlRpcMethod
+     * @return XmlRpcMethod|null
      */
-    public static function parse(string $payload): self
+    public static function parse(string $payload): self|null
     {
-        $parser = xml_parser_create();
-        xml_parse_into_struct($parser, $payload, $elements, $index);
-        xml_parser_free($parser);
+        $errorflag = libxml_use_internal_errors(true);
 
-        $in_param = false;
-        $method_name = null;
+        $xml = XMLReader::XML($payload);
 
-        foreach ($elements as $e) {
-            switch ($e['tag']) {
-                case 'METHODNAME':
-                    $method_name = $e['value'];
-                    break;
+        $method_name = null;
+        $params = [];
+        $expect = null;
 
-                case 'PARAM':
-                    if ($e['type'] === 'open') {
-                        $in_param = true;
+        while ($xml->read()) {
+            $tag = strtolower($xml->name);
+            switch ($tag) {
+                case 'methodname':
+                case 'value':
+                    if ($xml->nodeType === XMLReader::ELEMENT) {
+                        $expect = $tag;
                     } else {
-                        $in_param = false;
+                        $expect = null;
                     }
                     break;
 
-                case 'VALUE':
-                    if ($in_param) {
-                        $params[] = $e['value'];
+                case '#text':
+                    if ($xml->hasValue) {
+                        switch ($expect) {
+                            case 'methodname':
+                                $method_name = $xml->value;
+                                break;
+
+                            case 'value':
+                                $params[] = $xml->value;
+                                break;
+                        }
                     }
                     break;
 
@@ -60,7 +69,21 @@ class XmlRpcMethod
             }
         }
 
-        return new XmlRpcMethod($method_name, $params);
+        $errors = libxml_get_errors();
+        libxml_use_internal_errors($errorflag);
+
+        if (! empty($errors)) {
+            throw new \RuntimeException(
+                'errors while parsing XML',
+                $errors[0]->code
+            );
+        }
+
+        if (! empty($method_name)) {
+            return new XmlRpcMethod($method_name, $params);
+        }
+
+        return null;
     }
 
     /**
diff --git a/src/process-request.php b/src/process-request.php
index d0af0b5..69e1e58 100644
--- a/src/process-request.php
+++ b/src/process-request.php
@@ -29,13 +29,17 @@ $data = [
 ];
 
 if (preg_match('/xmlrpc\.php/i', $data['REQUEST_URI']) && $data['REQUEST_METHOD'] === 'POST') {
-    $method = XmlRpcMethod::parse($data['BODY']);
-    if ($method->name === 'wp.getUsersBlogs') {
-        save_credentials($data['REQUEST_TIME'], $data['REMOTE_ADDR'], $method->params[0], $method->params[1]);
-        error_log("Trapped XML-RPC request: saved credentials");
-
-        header("HTTP/1.1 404 Not Found");
-        die();
+    try {
+        $method = XmlRpcMethod::parse($data['BODY']);
+        if ($method && $method->name === 'wp.getUsersBlogs') {
+            save_credentials($data['REQUEST_TIME'], $data['REMOTE_ADDR'], $method->params[0], $method->params[1]);
+            error_log("Trapped XML-RPC request: saved credentials");
+
+            header("HTTP/1.1 404 Not Found");
+            die();
+        }
+    } catch (RuntimeException $e) {
+        error_log("Webtrap: {$e->getMessage()} ({$e->getCode()}), saving request instead.");
     }
 }
 
-- 
cgit v1.2.3