[imp] Horde v 5.2.22 vulnerability – obfuscation via HTML encoding – XSS payload

Mon Apr 14 16:17:29 UTC 2025

Hi,

On 24/03/2025 11:16, Jens Wahnes wrote:
> One solution I found to filter out the malicious content from emails like the one Nataša described was to tighten the code used to sanitize HTML in e-mails. This is found in the imp/lib/Mime/Viewer/Html.php file. The code in the big "switch" statement of the "_node" method, around line 435 or so, dealing with "case 'style'", can be extended to call "removeChild($node)" not only in the sub-case of 'text/css', as already present in the file, but also in the general case. 

NB:

the initial code
- is moving <style type="text/css"> tags as direct children of <head>, out of the HTML flow (and so out of <math>)
- is leaving <style> tags unchanged (hence the security issue)

the suggested change ( https://github.com/horde/imp/pull/15/commits/51c4173489477692527748f46d35b568df686868 )
- is completly ignoring <style> tags (only keeping <style type="text/css">)

I suggest also moving <style> with no "type" as direct children of <head>.
The resulting code:

         case 'style':
             switch (Horde_String::lower($node->getAttribute('type'))) {
             case '':
             case 'text/css':
                 $this->_imptmp['style'][] = str_replace(
                     array(''),
                     '',
                     $node->nodeValue
                 );
                 break;
             }
             if ($node->parentNode) {
                 $node->parentNode->removeChild($node);
             }
             break;

NB: according to spec https://html.spec.whatwg.org/multipage/semantics.html#the-style-element , <style> with other "type"s should "return" (ie be ignored?).



An alternative fix is to use <iframe> sandbox attribute to disallow "allow-scripts".
We have the following change for a subset of our users, we will see if it has no drawbacks...

--- lib/Compose/Link.php
+++ lib/Compose/Link.php
@@ -134,7 +134,7 @@ class IMP_Compose_Link
       */
      public function composeLinkSimpleCallback($url)
      {
-        return "javascript:void(window.open('" . strval($url) . "','','width=820,height=610,status=1,scrollbars=yes,resizable=yes'))";
+        return strval($url);
      }

      /**
--- lib/Mime/Viewer/Html.php
+++ lib/Mime/Viewer/Html.php
@@ -94,8 +94,9 @@ class IMP_Mime_Viewer_Html extends Horde_Mime_Viewer_Html
              if ($view == $registry::VIEW_SMARTMOBILE) {
                  $data['js'][] = '$("#imp-message-body a[href=\'#unblock-image\']").button()';
              }
+            $sandbox = 'sandbox="allow-downloads allow-popups allow-popups-to-escape-sandbox allow-same-origin"';

-            $data['data'] = '<div>' . _("Loading...") . '</div><iframe class="htmlMsgData" id="' . $uid . '" src="javascript:false" frameborder="0" style="display:none;height:auto;"></iframe>';
+            $data['data'] = '<div>' . _("Loading...") . '</div><iframe class="htmlMsgData" id="' . $uid . '" src="javascript:false" frameborder="0" style="display:none;height:auto;" '.$sandbox.'></iframe>';
              $data['type'] = 'text/html; charset=UTF-8';
              break;
          }
@@ -336,6 +337,7 @@ class IMP_Mime_Viewer_Html extends Horde_Mime_Viewer_Html
                      $clink = new IMP_Compose_Link($node->getAttribute('href'));
                      $node->setAttribute('href', $clink->link(true));
+                    $node->setAttribute('target', '_blank');
                  } elseif (!empty($this->_imptmp['inline']) &&
                            isset($url['fragment']) &&
                            empty($url['path']) &&

