Request 3245 (accepted)

We truncated the diff of some files because they were too big. If you want to see the full diff for every file, click here.

This request supersedes: request 3244 (Show diff)

Overview

Request 3245 (accepted)

1.4.15 with XSS patch

Created by mollekopf 6 months ago
In state accepted
Supersedes 3244
Open review for Admin
Open review for vanmeeuwen
Open review for mollekopf
Open review for kolab-developers

Submit roundcubemail

Submit package Kolab:16:Testing / roundcubemail to package Kolab:16 / roundcubemail

roundcubemail.spec Changed

@@ -48,7 +48,7 @@
 %global logdir /var/log/roundcubemail
 %global tmpdir /var/lib/roundcubemail
 
-%global rc_version 1.4.13
+%global rc_version 1.4.15
 #%%global rc_rel_suffix rc2.12
 %global dot_rel_suffix %{?rc_rel_suffix:.%{rc_rel_suffix}}
 %global dash_rel_suffix %{?rc_rel_suffix:-%{rc_rel_suffix}}
@@ -79,6 +79,7 @@
 Patch201:       default-configuration.patch
 Patch202:       roundcubemail-1.4-beta86-plugin-enigma-homedir.patch
 Patch203:       0001-Enigma-Optional-support-for-passwordless-keys-7265.patch
+Patch204:       0001-Fix-cross-site-scripting-XSS-vulnerability-in-settin.patch
 
 BuildArch:      noarch
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root%(%{__id_u} -n)
@@ -1536,6 +1537,7 @@
 %patch202 -p1
 %endif
 %patch203 -p1
+%patch204 -p1
 
 # Remove the results of patching when there's an incidental offset
 find . -type f -name "*.orig" | while read file; do
@@ -3450,6 +3452,9 @@
 %defattr(-,root,root,-)
 
 %changelog
+* Tue Nov  7 2023 Christian Mollekopf <mollekopf@apheleia-it.ch> - 1.4.15-2
+- Release of 1.4.15
+
 * Mon Mar  7 2022 Jeroen van Meeuwen <vanmeeuwen@apheleia-it.ch> - 1.4.13-2
 - Support passwordless keys

0001-Fix-cross-site-scripting-XSS-vulnerability-in-settin.patch Added

@@ -0,0 +1,241 @@
+From a5b44239022b34eaa034a4352b617974252f29a9 Mon Sep 17 00:00:00 2001
+From: Christian Mollekopf <mollekopf@apheleia-it.ch>
+Date: Tue, 7 Nov 2023 10:53:14 +0100
+Subject: PATCH Fix cross-site scripting (XSS) vulnerability in setting
+ Content-Type/Content-Disposition for attachment preview/download
+
+Thanks to rehme.infosec for reporting the issues.
+---
+ CHANGELOG                               |  4 ++
+ program/lib/Roundcube/rcube_charset.php | 12 ++++++
+ program/lib/Roundcube/rcube_imap.php    |  5 +++
+ program/lib/Roundcube/rcube_output.php  | 53 ++++++++++++++++++-------
+ program/steps/mail/viewsource.inc       | 18 +++++----
+ tests/Framework/Charset.php             | 30 +++++++++++++-
+ 6 files changed, 99 insertions(+), 23 deletions(-)
+
+diff --git a/CHANGELOG b/CHANGELOG
+index bcaa870e3..ead3d106c 100644
+--- a/CHANGELOG
++++ b/CHANGELOG
+@@ -1,5 +1,9 @@
+ # Changelog Roundcube Webmail
+ 
++- Fix cross-site scripting (XSS) vulnerability in setting Content-Type/Content-Disposition for attachment preview/download
++
++RELEASE 1.4.15
++--------------
+ - Fix cross-site scripting (XSS) vulnerability in handling of SVG in HTML messages (#9168)
+ - Fix PHP 5.4 compatibility by using pear-core-minimal 1.10.11 (#9148)
+ 
+diff --git a/program/lib/Roundcube/rcube_charset.php b/program/lib/Roundcube/rcube_charset.php
+index 88eb6001f..1f69e118d 100644
+--- a/program/lib/Roundcube/rcube_charset.php
++++ b/program/lib/Roundcube/rcube_charset.php
+@@ -182,6 +182,18 @@ class rcube_charset
+         throw new ErrorException($errstr, 0, $errno);
+     }
+ 
++    /**
++     * Validate character set identifier.
++     *
++     * @param string $input Character set identifier
++     *
++     * @return bool True if valid, False if not valid
++     */
++    public static function is_valid($input)
++    {
++        return is_string($input) && preg_match('|^a-zA-Z0-9_./:#-{2,32}$|', $input) > 0;
++    }
++
+     /**
+      * Parse and validate charset name string.
+      * Sometimes charset string is malformed, there are also charset aliases,
+diff --git a/program/lib/Roundcube/rcube_imap.php b/program/lib/Roundcube/rcube_imap.php
+index 714ba33f3..c1eebe1ba 100644
+--- a/program/lib/Roundcube/rcube_imap.php
++++ b/program/lib/Roundcube/rcube_imap.php
+@@ -2129,6 +2129,11 @@ class rcube_imap extends rcube_storage
+             $struct->charset = $mime_headers->charset;
+         }
+ 
++        // Sanitize charset for security
++        if ($struct->charset && !rcube_charset::is_valid($struct->charset)) {
++            $struct->charset = '';
++        }
++
+         // read content encoding
+         if (!empty($part5)) {
+             $struct->encoding = strtolower($part5);
+diff --git a/program/lib/Roundcube/rcube_output.php b/program/lib/Roundcube/rcube_output.php
+index 27acc7075..f24c47fda 100644
+--- a/program/lib/Roundcube/rcube_output.php
++++ b/program/lib/Roundcube/rcube_output.php
+@@ -212,7 +212,7 @@ abstract class rcube_output
+     }
+ 
+     /**
+-     * Send headers related to file downloads
++     * Send headers related to file downloads.
+      *
+      * @param string $filename File name
+      * @param array  $params   Optional parameters:
+@@ -225,31 +225,54 @@ abstract class rcube_output
+      */
+     public function download_headers($filename, $params = array())
+     {
++        // For security reasons we validate type, filename and charset params.
++        // Some HTTP servers might drop a header that is malformed or very long, this then
++        // can lead to web browsers unintentionally executing javascript code in the body.
++
+         if (empty($params'disposition')) {
+             $params'disposition' = 'attachment';
+         }
+ 
+-        if ($params'disposition' == 'inline' && stripos($params'type', 'text') === 0) {
+-            $params'type' .= '; charset=' . ($params'type_charset' ?: $this->charset);
+-        }
+-
+-        header("Content-Type: " . ($params'type' ?: "application/octet-stream"));
++        $ctype       = 'application/octet-stream';
++        $disposition = $params'disposition';
+ 
+-        if ($params'disposition' == 'attachment' && $this->browser->ie) {
+-            header("Content-Type: application/force-download");
++        if (!empty($params'type') && is_string($params'type') && strlen($params'type') < 256
++            && preg_match('/^a-z0-9!#$&.+^_-+\/a-z0-9!#$&.+^_-+$/i', $params'type')
++        ) {
++            $ctype = $params'type';
+         }
+ 
+-        $disposition = "Content-Disposition: " . $params'disposition';
++        if ($disposition == 'inline' && stripos($ctype, 'text') === 0) {
++            $charset = $this->charset;
++            if (!empty($params'type_charset') && rcube_charset::is_valid($params'type_charset')) {
++                $charset = $params'type_charset';
++            }
+ 
+-        // For non-ascii characters we'll use RFC2231 syntax
+-        if (!preg_match('/^a-zA-Z0-9_.:,?;@+ -/', $filename)) {
+-            $disposition .= sprintf("; filename=\"%s\"", $filename);
++            $ctype .= "; charset={$charset}";
+         }
+-        else {
+-            $disposition .= sprintf("; filename*=%s''%s", $params'charset' ?: $this->charset, rawurlencode($filename));
++
++        if (is_string($filename) && strlen($filename) > 0 && strlen($filename) <= 1024) {
++            // For non-ascii characters we'll use RFC2231 syntax
++            if (!preg_match('/^a-zA-Z0-9_.:,?;@+ -/', $filename)) {
++                $disposition .= "; filename=\"{$filename}\"";
++            }
++            else {
++                $filename = rawurlencode($filename);
++                $charset  = $this->charset;
++                if (!empty($params'charset') && rcube_charset::is_valid($params'charset')) {
++                    $charset = $params'charset';
++                }
++
++                $disposition .= "; filename*={$charset}''{$filename}";
++            }
+         }
+ 
+-        header($disposition);
++        header("Content-Disposition: {$disposition}");
++        header("Content-Type: {$ctype}");
++
++        if ($params'disposition' == 'attachment' && $this->browser->ie) {
++            header("Content-Type: application/force-download");
++        }
+ 
+         if (isset($params'length')) {
+             header("Content-Length: " . $params'length');
+diff --git a/program/steps/mail/viewsource.inc b/program/steps/mail/viewsource.inc
+index 532ea073b..91b1703e0 100644
+--- a/program/steps/mail/viewsource.inc
++++ b/program/steps/mail/viewsource.inc
+@@ -34,23 +34,27 @@ if ($uid = rcube_utils::get_input_value('_uid', rcube_utils::INPUT_GET)) {
+         $headers = $RCMAIL->storage->get_message_headers($uid);
+     }
+ 
+-    $charset = $headers->charset ?: $RCMAIL->config->get('default_charset');
++    $charset  = $headers->charset ?: $RCMAIL->config->get('default_charset');
++    $filename = '';
++    $params   = array(
++        'type'         => 'text/plain',
++        'type_charset' => $charset,
++    );
+ 
+     if (!empty($_GET'_save')) {
+         $subject  = rcube_mime::decode_header($headers->subject, $headers->charset);
+         $filename = rcmail_filename_from_subject(mb_substr($subject, 0, 128));
+         $filename = ($filename ?: $uid)  . '.eml';
+ 
+-        $RCMAIL->output->download_headers($filename, array(
+-                'length'       => $headers->size,
+-                'type'         => 'text/plain',
+-                'type_charset' => $charset,
+-        ));
++        $params'length' = $headers->size;
++        $params'disposition' = 'attachment';
+     }
+     else {
+-        header("Content-Type: text/plain; charset={$charset}");
++        $params'disposition' = 'inline';
+     }
+ 
++    $RCMAIL->output->download_headers($filename, $params);
++
+     if (isset($message)) {
+         $message->get_part_body($part_id, empty($_GET'_save'), 0, -1);
+     }
+diff --git a/tests/Framework/Charset.php b/tests/Framework/Charset.php
+index f36b7b969..af6c4c783 100644
+--- a/tests/Framework/Charset.php
++++ b/tests/Framework/Charset.php
+@@ -9,7 +9,6 @@
+  */
+ class Framework_Charset extends PHPUnit_Framework_TestCase
+ {
+-
+     /**
+      * Data for test_clean()
+      */
+@@ -40,6 +39,35 @@ class Framework_Charset extends PHPUnit_Framework_TestCase
+         $this->assertNotRegExp('/\xD0\xD0/', rcube_charset::clean($bogus));
+     }
+ 
++    /**
++     * Data for test_is_valid()
++     */
++    function data_is_valid()
++    {
++        $list = ;
++        foreach (mb_list_encodings() as $charset) {
++            $list = $charset, true;
++        }
++
++        return array_merge($list, 
++            '', false,
++            'a', false,
++            'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', false,
++            null, false,
++
++            'TCVN5712-1:1993', true,
++            'JUS_I.B1.002', true,
++        );
++    }
++
++    /**
++     * @dataProvider data_is_valid
++     */
++    function test_is_valid($input, $result)
++    {
++        $this->assertSame($result, rcube_charset::is_valid($input));
++    }
++
+     /**
+      * Data for test_parse_charset()
+      */
+-- 
+2.41.0
+

debian.changelog Changed

debian.series Changed

roundcubemail-1.4.13.tar.gz -> roundcubemail-1.4.15.tar.gz Changed

roundcubemail.dsc Changed

Build Results
RPM Lint

Refresh

Comments for request 3245 0

Request History

mollekopf created request 6 months ago

1.4.15 with XSS patch

mollekopf accepted request 6 months ago