From 01246059180bd9f2946d49f4fb939852f697ce17 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Sun, 27 Mar 2011 21:24:32 +0100 Subject: [PATCH] Fix CSS URL innerHTML/cssText escaping bug. Signed-off-by: Edward Z. Yang --- NEWS | 5 +++-- library/HTMLPurifier/AttrDef/CSS/URI.php | 9 +++++++++ tests/HTMLPurifier/AttrDef/CSS/URITest.php | 4 ++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 8dd7d144..720db5a0 100644 --- a/NEWS +++ b/NEWS @@ -20,8 +20,9 @@ NEWS ( CHANGELOG and HISTORY ) HTMLPurifier reverted using %Output.FixInnerHTML. Reported by Neike Taika-Tessaro and Mario Heiderich. # Protect against cssText/innerHTML by restricting allowed characters - used in fonts further than mandated by the specification. Reported - by Neike Taika-Tessaro and Mario Heiderich. + used in fonts further than mandated by the specification and encoding + some extra special characters in URLs. Reported by Neike + Taika-Tessaro and Mario Heiderich. ! Added %HTML.Nofollow to add rel="nofollow" to external links. ! More types of SPL autoloaders allowed on later versions of PHP. ! Implementations for position, top, left, right, bottom, z-index diff --git a/library/HTMLPurifier/AttrDef/CSS/URI.php b/library/HTMLPurifier/AttrDef/CSS/URI.php index 1df17dc2..c2f767e5 100644 --- a/library/HTMLPurifier/AttrDef/CSS/URI.php +++ b/library/HTMLPurifier/AttrDef/CSS/URI.php @@ -43,6 +43,15 @@ class HTMLPurifier_AttrDef_CSS_URI extends HTMLPurifier_AttrDef_URI // extra sanity check; should have been done by URI $result = str_replace(array('"', "\\", "\n", "\x0c", "\r"), "", $result); + // suspicious characters are ()'; we're going to percent encode + // them for safety. + $result = str_replace(array('(', ')', "'"), array('%28', '%29', '%27'), $result); + + // there's an extra bug where ampersands lose their escaping on + // an innerHTML cycle, so a very unlucky query parameter could + // then change the meaning of the URL. Unfortunately, there's + // not much we can do about that... + return "url(\"$result\")"; } diff --git a/tests/HTMLPurifier/AttrDef/CSS/URITest.php b/tests/HTMLPurifier/AttrDef/CSS/URITest.php index 836f1034..3d6f5791 100644 --- a/tests/HTMLPurifier/AttrDef/CSS/URITest.php +++ b/tests/HTMLPurifier/AttrDef/CSS/URITest.php @@ -20,8 +20,8 @@ class HTMLPurifier_AttrDef_CSS_URITest extends HTMLPurifier_AttrDefHarness $this->assertDef("url('http://www.example.com/')", $result); $this->assertDef( ' url( "http://www.example.com/" ) ', $result); - $this->assertDef("url(http://www.example.com/foo,bar\))", - 'url("http://www.example.com/foo,bar)")'); + $this->assertDef("url(http://www.example.com/foo,bar\)\'\()", + 'url("http://www.example.com/foo,bar%29%27%28")'); } }