From 04994005e7f83a9ae48bd177b898dd17f7aa8e24 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Tue, 13 Jan 2015 09:41:41 +0100 Subject: Fix XSS issue in style attribute handling (#1490227) Conflicts: CHANGELOG --- CHANGELOG | 1 + program/lib/Roundcube/rcube_washtml.php | 4 ++-- tests/Framework/Washtml.php | 24 +++++++++++++++++++++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e046f5e88..39597be63 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ CHANGELOG Roundcube Webmail - Fix wrong icon for download button in classic skin - Fix bug where sent message was saved in Sent folder even if disabled by user (#1490208) - Fix checks based on window.ActiveXObject in IE > 10 +- Fix XSS issue in style attribute handling (#1490227) RELEASE 1.0.4 ------------- diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php index 984294376..630a86e01 100644 --- a/program/lib/Roundcube/rcube_washtml.php +++ b/program/lib/Roundcube/rcube_washtml.php @@ -243,8 +243,8 @@ class rcube_washtml $t .= ' ' . $key . '="' . htmlspecialchars($value, ENT_QUOTES) . '"'; } else if ($key == 'style' && ($style = $this->wash_style($value))) { - $quot = strpos($style, '"') !== false ? "'" : '"'; - $t .= ' style=' . $quot . $style . $quot; + // replace double quotes to prevent syntax error and XSS issues (#1490227) + $t .= ' style="' . str_replace('"', '"', $style) . '"'; } else if ($key == 'background' || ($key == 'src' && strtolower($node->tagName) == 'img')) { //check tagName anyway if (($src = $this->config['cid_map'][$value]) diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php index f041504d7..e4e3de41f 100644 --- a/tests/Framework/Washtml.php +++ b/tests/Framework/Washtml.php @@ -159,7 +159,7 @@ class Framework_Washtml extends PHPUnit_Framework_TestCase $washer = new rcube_washtml; $washed = $washer->wash($html); - $this->assertRegExp('|style=\'font-family: "新細明體","serif"; color: red\'|', $washed, "Unicode chars in style attribute - quoted (#1489697)"); + $this->assertRegExp('|style="font-family: \"新細明體\",\"serif\"; color: red"|', $washed, "Unicode chars in style attribute - quoted (#1489697)"); $html = " test"; @@ -183,4 +183,26 @@ class Framework_Washtml extends PHPUnit_Framework_TestCase $this->assertRegExp('|line-height: 1;|', $washed, "Untouched line-height (#1489917)"); $this->assertRegExp('|; height: 10px|', $washed, "Fixed height units"); } + + /** + * Test invalid style cleanup - XSS prevention (#1490227) + */ + function test_style_wash_xss() + { + $html = ""; + $exp = ""; + + $washer = new rcube_washtml; + $washed = $washer->wash($html); + + $this->assertTrue(strpos($washed, $exp) !== false, "Style quotes XSS issue (#1490227)"); + + $html = ""; + $exp = ""; + + $washer = new rcube_washtml; + $washed = $washer->wash($html); + + $this->assertTrue(strpos($washed, $exp) !== false, "Style quotes XSS issue (#1490227)"); + } } -- cgit v1.2.3