From 786aa0725e28976bed5037814e01c44c90a551f5 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Tue, 13 Jan 2015 03:41:41 -0500
Subject: [PATCH] Fix XSS issue in style attribute handling (#1490227)
---
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 3029af2..edfc13d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@
- Fix bug where max_group_members was ignored when adding a new contact (#1490214)
- Hide MDN and DSN options in compose if disabled by admin (#1490221)
- Fix checks based on window.ActiveXObject in IE > 10
+- Fix XSS issue in style attribute handling (#1490227)
RELEASE 1.1-rc
--------------
diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php
index 4bf189e..e0cce68 100644
--- a/program/lib/Roundcube/rcube_washtml.php
+++ b/program/lib/Roundcube/rcube_washtml.php
@@ -244,8 +244,8 @@
$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' && preg_match('/^(img|source)$/i', $node->tagName))
diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php
index f041504..e4e3de4 100644
--- a/tests/Framework/Washtml.php
+++ b/tests/Framework/Washtml.php
@@ -159,7 +159,7 @@
$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 = "<html><meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\" />
<body><span style='font-family:新細明體;color:red'>test</span></body></html>";
@@ -183,4 +183,26 @@
$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 = "<img style=aaa:'\"/onerror=alert(1)//'>";
+ $exp = "<img style=\"aaa: '"/onerror=alert(1)//'\" />";
+
+ $washer = new rcube_washtml;
+ $washed = $washer->wash($html);
+
+ $this->assertTrue(strpos($washed, $exp) !== false, "Style quotes XSS issue (#1490227)");
+
+ $html = "<img style=aaa:'"/onerror=alert(1)//'>";
+ $exp = "<img style=\"aaa: '"/onerror=alert(1)//'\" />";
+
+ $washer = new rcube_washtml;
+ $washed = $washer->wash($html);
+
+ $this->assertTrue(strpos($washed, $exp) !== false, "Style quotes XSS issue (#1490227)");
+ }
}
--
Gitblit v1.9.1