Merge pull request #184 from JWardee/v2.1.3

V2.1.3
JWardee · Jun 9, 2023 · 8eab103 · 8eab103
2 parents 86f7120 + cd3f1df
commit 8eab103
Show file tree

Hide file tree

Showing 12 changed files with 70 additions and 70 deletions.
diff --git a/WpMailCatcher.php b/WpMailCatcher.php
@@ -6,7 +6,7 @@
 Domain Path: /languages
 Description: Logging your mail will stop you from ever losing your emails again! This fast, lightweight plugin (under 140kb in size!) is also useful for debugging or backing up your messages.
 Author: James Ward
-Version: 2.1.2
+Version: 2.1.3
 Author URI: https://jamesward.io
 Donate link: https://paypal.me/jamesmward
 */

diff --git a/build/grunt/package.json b/build/grunt/package.json
@@ -1,6 +1,6 @@
 {
   "name": "WpMailCatcher",
-  "version": "2.1.1",
+  "version": "2.1.3",
   "lang_po_directory": "../../languages",
   "build_directory": "./..",
   "dist_directory": "../../assets",

diff --git a/readme.txt b/readme.txt
@@ -2,9 +2,9 @@
 Contributors: Wardee
 Tags: mail logging, email log, email logger, logging, email logging, mail, crm
 Requires at least: 4.7
-Tested up to: 6.2.2
+Tested up to: 6.2.3
 Requires PHP: 7.4
-Stable tag: 2.1.2
+Stable tag: 2.1.3
 License: GNU General Public License v3.0
 License URI: https://raw.githubusercontent.com/JWardee/wp-mail-catcher/master/LICENSE
 Donate link: https://paypal.me/jamesmward
@@ -94,6 +94,11 @@ Great! Please leave a note in our (GitHub tracker)
 
 == Changelog ==
 
+= 2.1.3 =
+
+- Fix: Improved HTML email detection
+- Fix: Improved XSS filtering
+
 = 2.1.2 =
 
 - Fix: Escaping no longer mangles exports

diff --git a/src/GeneralHelper.php b/src/GeneralHelper.php
@@ -27,7 +27,7 @@ class GeneralHelper
     public static $namespacePrefix;
     public static $reviewLink;
     public static $actionNameSpace;
-    public static $htmlEmailHeader = 'content-type: text/html;';
+    public static $htmlEmailHeader = 'content-type: text/html';
 
     public static function setSettings()
     {
@@ -132,7 +132,7 @@ public static function labelToSlug($label)
         return strtolower($label);
     }
 
-    public static function sanitiseForQuery($value)
+    public static function sanitiseForDbQuery($value)
     {
         switch (gettype($value)) {
             case ('array'):
@@ -148,14 +148,16 @@ public static function sanitiseForQuery($value)
         return $value;
     }
 
-    public static function sanitiseHtmlspecialchars($input): string
+    private static function getAllowedTags()
     {
-        return htmlspecialchars(
-            $input,
-            ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401,
-            null,
-            false
-        );
+        $tags = wp_kses_allowed_html('post');
+        $tags['style'] = [];
+        return $tags;
+    }
+
+    public static function filterHtml($value)
+    {
+        return wp_kses($value, self::getAllowedTags());
     }
 
     public static function getAttachmentIdsFromUrl($urls)
@@ -166,7 +168,7 @@ public static function getAttachmentIdsFromUrl($urls)
 
         global $wpdb;
 
-        $urls = self::sanitiseForQuery($urls);
+        $urls = self::sanitiseForDbQuery($urls);
 
         $sql = "SELECT DISTINCT post_id
                 FROM " . $wpdb->prefix . "postmeta

diff --git a/src/Loggers/BuddyPress.php b/src/Loggers/BuddyPress.php
@@ -44,9 +44,9 @@ protected function getTransformedMailArgs(object $bpMail): array
 
         return [
             'time' => time(),
-            'email_to' => GeneralHelper::arrayToString($tos),
-            'subject' => $bpMail->get_subject(),
-            'message' => $this->sanitiseInput($bpMail->get_content()),
+            'email_to' => GeneralHelper::filterHtml(GeneralHelper::arrayToString($tos)),
+            'subject' => GeneralHelper::filterHtml($bpMail->get_subject()),
+            'message' => GeneralHelper::filterHtml($bpMail->get_content()),
             'backtrace_segment' => json_encode($this->getBacktrace('bp_send_email')),
             'status' => 1,
             'attachments' => '',//json_encode($this->getAttachmentLocations($args['attachments'])),

diff --git a/src/Loggers/LogHelper.php b/src/Loggers/LogHelper.php
@@ -142,25 +142,6 @@ protected function getAttachmentLocations($attachments): array
         return $result;
     }
 
-    protected function sanitiseInput($input): string
-    {
-        return htmlspecialchars(
-            $input,
-            ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401,
-            null,
-            false
-        );
-    }
-
-    protected function sanitiseAndRemoveScripts($input): string
-    {
-        return preg_replace(
-            '#<script(.*?)>(.*?)</script>#is',
-            '',
-            GeneralHelper::sanitiseHtmlspecialchars($input)
-        );
-    }
-
     /**
      * Get the details of the method that originally triggered wp_mail
      *

diff --git a/src/Loggers/WpMail.php b/src/Loggers/WpMail.php
@@ -42,9 +42,9 @@ protected function getTransformedMailArgs(array $args): array
     {
         return [
             'time' => time(),
-            'email_to' => $this->sanitiseInput(GeneralHelper::arrayToString($args['to'])),
-            'subject' => $this->sanitiseInput($args['subject']),
-            'message' => $this->sanitiseAndRemoveScripts($args['message']),
+            'email_to' => GeneralHelper::filterHtml(GeneralHelper::arrayToString($args['to'])),
+            'subject' => GeneralHelper::filterHtml($args['subject']),
+            'message' => GeneralHelper::filterHtml($args['message']),
             'backtrace_segment' => json_encode($this->getBacktrace()),
             'status' => 1,
             'attachments' => json_encode($this->getAttachmentLocations($args['attachments'])),

diff --git a/src/MailAdminTable.php b/src/MailAdminTable.php
@@ -31,6 +31,18 @@ public static function getInstance()
         return self::$instance;
     }
 
+    private function runHtmlSpecialChars($value)
+    {
+        $value = GeneralHelper::filterHtml($value);
+
+        return htmlspecialchars(
+            $value,
+            ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401,
+            null,
+            false
+        );
+    }
+
     function column_default($item, $column_name)
     {
         switch ($column_name) {
@@ -58,7 +70,8 @@ function column_subject($item)
             );
 
             $subjectDecoded = base64_decode($subjectEncoded);
-            $subjectDecoded = GeneralHelper::sanitiseHtmlspecialchars($subjectDecoded);
+            $subjectDecoded = $this->runHtmlSpecialChars($subjectDecoded);
+
             return '<span class="asci-help" data-hover-message="' . __("This subject was base64 decoded") . '">
                         <a href="' . $this->asciSubjectHelpLink . '" target="_blank">(?)</a>
                         ' . $subjectDecoded . '
@@ -74,14 +87,15 @@ function column_subject($item)
 
             $subjectDecoded = quoted_printable_decode($subjectEncoded);
             $subjectDecoded = base64_decode($subjectEncoded);
-            $subjectDecoded = GeneralHelper::sanitiseHtmlspecialchars($subjectDecoded);
+            $subjectDecoded = $this->runHtmlSpecialChars($subjectDecoded);
+
             return '<span class="asci-help" data-hover-message="' . __("This subject was quoted printable decoded") . '">
                         <a href="' . $this->asciSubjectHelpLink . '" target="_blank">(?)</a>
                         ' . $subjectDecoded . '
                     </span>';
         }
 
-        return GeneralHelper::sanitiseHtmlspecialchars($subject);
+        return $this->runHtmlSpecialChars($subject);
     }
 
     function column_time($item): string
@@ -125,7 +139,9 @@ function column_email_to($item): string
             'view' => '<a href="#" data-toggle="modal" data-target="#' . $item['id'] . '">' . __('View', 'WpMailCatcher') . '</a>',
         ];
 
-        return sprintf('%1$s %2$s', GeneralHelper::sanitiseHtmlspecialchars($item['email_to']), $this->row_actions($actions));
+        $emailTo = $this->runHtmlSpecialChars($item['email_to']);
+
+        return sprintf('%1$s %2$s', $emailTo, $this->row_actions($actions));
     }
 
     function column_status($item): string

diff --git a/src/Models/Logs.php b/src/Models/Logs.php
@@ -84,7 +84,7 @@ public static function get(array $args = [])
         /**
          * Sanitise each value in the array
          */
-        array_walk_recursive($args, 'WpMailCatcher\GeneralHelper::sanitiseForQuery');
+        array_walk_recursive($args, 'WpMailCatcher\GeneralHelper::sanitiseForDbQuery');
 
         $sql = "SELECT " . implode(',', $columnsToSelect) . "
             FROM " . $wpdb->prefix . GeneralHelper::$tableName . " ";
@@ -180,23 +180,11 @@ private static function dbResultTransform($results, $args = [])
             // Otherwise resort to the original method
             } elseif (isset($result['additional_headers'])) {
                 $result['is_html'] = GeneralHelper::doesArrayContainSubString(
-                    $result['additional_headers'],
-                    GeneralHelper::$htmlEmailHeader
+                    str_replace(' ', '', $result['additional_headers']),
+                    str_replace(' ', '', GeneralHelper::$htmlEmailHeader)
                 );
             }
 
-            if (isset($result['message'])) {
-                $result['message'] = stripslashes(htmlspecialchars_decode($result['message']));
-            }
-
-            if (isset($result['subject'])) {
-                $result['subject'] = stripslashes(htmlspecialchars_decode($result['subject']));
-            }
-
-            if (isset($result['email_to'])) {
-                $result['email_to'] = stripslashes(htmlspecialchars_decode($result['email_to']));
-            }
-
             if (!empty($result['attachments'])) {
                 $result['attachments'] = json_decode($result['attachments'], true);
 
@@ -238,7 +226,7 @@ public static function delete($ids)
         global $wpdb;
 
         $ids = GeneralHelper::arrayToString($ids);
-        $ids = GeneralHelper::sanitiseForQuery($ids);
+        $ids = GeneralHelper::sanitiseForDbQuery($ids);
 
         $wpdb->query("DELETE FROM " . $wpdb->prefix . GeneralHelper::$tableName . "
                       WHERE id IN(" . $ids . ")");

diff --git a/src/Models/Mail.php b/src/Models/Mail.php
@@ -21,6 +21,14 @@ public static function resend($ids)
 
             add_filter('wp_mail_content_type', $updateContentType, self::$contentTypeFilterPriority);
 
+            if (isset($log['message'])) {
+                $log['message'] = GeneralHelper::filterHtml($log['message']);
+            }
+
+            if (isset($log['subject'])) {
+                $log['subject'] = GeneralHelper::filterHtml($log['subject']);
+            }
+
             wp_mail(
                 $log['email_to'],
                 $log['subject'],
@@ -54,6 +62,14 @@ public static function export($ids, $forceBrowserDownload = true)
                 return in_array($key, GeneralHelper::$csvExportLegalColumns);
             }, ARRAY_FILTER_USE_KEY);
 
+            if (isset($log['message'])) {
+                $log['message'] = GeneralHelper::filterHtml($log['message']);
+            }
+
+            if (isset($log['subject'])) {
+                $log['subject'] = GeneralHelper::filterHtml($log['subject']);
+            }
+
             if (isset($log['attachments']) && !empty($log['attachments']) && is_array($log['attachments'])) {
                 $log['attachments'] = array_column($log['attachments'], 'url');
                 $log['attachments'] = GeneralHelper::arrayToString(

diff --git a/src/Views/HtmlMessage.php b/src/Views/HtmlMessage.php
@@ -1,3 +1,5 @@
 <?php
 
-echo $log['message'] ?? '';
+use WpMailCatcher\GeneralHelper;
+
+echo GeneralHelper::filterHtml($log['message'] ?? '');
diff --git a/testing/tests/TestLogFunctions.php b/testing/tests/TestLogFunctions.php
@@ -437,14 +437,4 @@ public function testCanDecodeAsciQuotedEncodedSubjectLine()
             preg_replace('/\s+/', '', $expectedOutput)
         );
     }
-
-    public function testSubjectLineHtmlIsEscaped()
-    {
-        $mailTable = MailAdminTable::getInstance();
-        $subjectBase = '<script>alert("Hello");</script>';
-        $escapedSubject = GeneralHelper::sanitiseHtmlspecialchars($subjectBase);
-        $subject = $mailTable->column_subject(['subject' => $subjectBase]);
-
-        $this->assertEquals($subject, $escapedSubject);
-    }
 }