Request 3247 (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.

You're not reviewing the full diff of request 3247 , but the diff to the superseded request 3246 (Show full diff)

Overview

Request 3247 (accepted)

Added missing js deps

Created by mollekopf about 1 year ago
In state accepted
Supersedes 3246
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

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.15.tar.gz Changed

roundcubemail.dsc Changed

Build Results
RPM Lint

Refresh

Comments for request 3247 0

Request History

mollekopf created request about 1 year ago

Added missing js deps

mollekopf accepted request about 1 year ago