Merge lp:~ari-tczew/ubuntu/lucid/php-htmlpurifier/security into lp:ubuntu/lucid/php-htmlpurifier

[Artur Rona]

[Jamie Strandboge (jdstrand)]

Patch not the same as the tested fix in the bug.

review: Needs Fixing

[lp:~ari-tczew/ubuntu/lucid/php-htmlpurifier/security]

7. By Artur Rona on 2010-11-24

Remove done changes due to not correctly with packaging policy.

8. By Artur Rona on 2010-11-24

Patch files directly.

[Artur Rona (ari-tczew)]

I updated branches. Files have been patches directly, as correct with packaging policy. Bzr diff looks like not exactly with original patch, dunno why. I'll attach also debdiff.
Changes are the same such as François tested, but without using patchsystem. Packages built fine on my PPA.

review: Needs Resubmitting

Unmerged revisions

8. By Artur Rona on 2010-11-24

Patch files directly.

7. By Artur Rona on 2010-11-24

Remove done changes due to not correctly with packaging policy.

6. By Artur Rona on 2010-07-08

* SECURITY UPDATE (LP: #582576). More information on:
  - http://secunia.com/advisories/39613/
* debian/patches/01-fix_XSS_IE.dpatch: Cross-Site
  Scripting Vulnerability
  - SA39613
* debian/{control,rules}: Add support for dpatch.

=== modified file 'debian/changelog'
--- debian/changelog	2010-03-16 14:04:20 +0000
+++ debian/changelog	2010-11-24 22:47:32 +0000
@@ -1,3 +1,13 @@
+php-htmlpurifier (4.0.0+dfsg1-1ubuntu0.1) lucid-security; urgency=low
+
+  * SECURITY UPDATE (LP: #582576).
+  * A vulnerability has been reported in HTML Purifier,  which can be
+    exploited by malicious people to conduct cross-site scripting
+    attacks.
+  * CVE-2010-2479
+
+ -- Artur Rona <[e-mail]>  Wed, 24 Nov 2010 22:36:10 +0100
+
 php-htmlpurifier (4.0.0+dfsg1-1) unstable; urgency=low
 
   * Take original upstream tarball, removing non-DFSG-free XHTML

=== modified file 'debian/control'
--- debian/control	2010-03-16 14:04:20 +0000
+++ debian/control	2010-11-24 22:47:32 +0000
@@ -1,7 +1,8 @@
 Source: php-htmlpurifier
 Section: php
 Priority: optional
-Maintainer: Christian Bayle <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Christian Bayle <[e-mail]>
 Uploaders: Thorsten Glaser <[e-mail]>, Roland Mas <[e-mail]>
 Homepage: http://htmlpurifier.org/
 Build-Depends: debhelper (>= 5)

=== modified file 'library/HTMLPurifier/AttrDef.php'
--- library/HTMLPurifier/AttrDef.php	2010-03-16 14:04:20 +0000
+++ library/HTMLPurifier/AttrDef.php	2010-11-24 22:47:32 +0000
@@ -82,6 +82,42 @@
         return preg_replace('/rgb\((\d+)\s*,\s*(\d+)\s*,\s*(\d+)\)/', 'rgb(\1,\2,\3)', $string);
     }
 
+    /**
+     * Parses a possibly escaped CSS string and returns the "pure" 
+     * version of it.
+     */
+    protected function expandCSSEscape($string) {
+        // flexibly parse it
+        $ret = '';
+        for ($i = 0, $c = strlen($string); $i < $c; $i++) {
+            if ($string[$i] === '\\') {
+                $i++;
+                if ($i >= $c) {
+                    $ret .= '\\';
+                    break;
+                }
+                if (ctype_xdigit($string[$i])) {
+                    $code = $string[$i];
+                    for ($a = 1, $i++; $i < $c && $a < 6; $i++, $a++) {
+                        if (!ctype_xdigit($string[$i])) break;
+                        $code .= $string[$i];
+                    }
+                    // We have to be extremely careful when adding
+                    // new characters, to make sure we're not breaking
+                    // the encoding.
+                    $char = HTMLPurifier_Encoder::unichr(hexdec($code));
+                    if (HTMLPurifier_Encoder::cleanUTF8($char) === '') continue;
+                    $ret .= $char;
+                    if ($i < $c && trim($string[$i]) !== '') $i--;
+                    continue;
+                }
+                if ($string[$i] === "\n") continue;
+            }
+            $ret .= $string[$i];
+        }
+        return $ret;
+    }
+
 }
 
 // vim: et sw=4 sts=4

=== modified file 'library/HTMLPurifier/AttrDef/CSS/FontFamily.php'
--- library/HTMLPurifier/AttrDef/CSS/FontFamily.php	2010-03-16 14:04:20 +0000
+++ library/HTMLPurifier/AttrDef/CSS/FontFamily.php	2010-11-24 22:47:32 +0000
@@ -34,37 +34,10 @@
                 $quote = $font[0];
                 if ($font[$length - 1] !== $quote) continue;
                 $font = substr($font, 1, $length - 2);
-
-                $new_font = '';
-                for ($i = 0, $c = strlen($font); $i < $c; $i++) {
-                    if ($font[$i] === '\\') {
-                        $i++;
-                        if ($i >= $c) {
-                            $new_font .= '\\';
-                            break;
-                        }
-                        if (ctype_xdigit($font[$i])) {
-                            $code = $font[$i];
-                            for ($a = 1, $i++; $i < $c && $a < 6; $i++, $a++) {
-                                if (!ctype_xdigit($font[$i])) break;
-                                $code .= $font[$i];
-                            }
-                            // We have to be extremely careful when adding
-                            // new characters, to make sure we're not breaking
-                            // the encoding.
-                            $char = HTMLPurifier_Encoder::unichr(hexdec($code));
-                            if (HTMLPurifier_Encoder::cleanUTF8($char) === '') continue;
-                            $new_font .= $char;
-                            if ($i < $c && trim($font[$i]) !== '') $i--;
-                            continue;
-                        }
-                        if ($font[$i] === "\n") continue;
-                    }
-                    $new_font .= $font[$i];
-                }
-
-                $font = $new_font;
             }
+
+            $font = $this->expandCSSEscape($font);
+
             // $font is a pure representation of the font name
 
             if (ctype_alnum($font) && $font !== '') {
@@ -73,12 +46,21 @@
                 continue;
             }
 
+            // bugger out on whitespace.  form feed (0C) really
+            // shouldn't show up regardless
+            $font = str_replace(array("\n", "\t", "\r", "\x0C"), ' ', $font);
+
+            // These ugly transforms don't pose a security
+            // risk (as \\ and \" might).  We could try to be clever and
+            // use single-quote wrapping when there is a double quote
+            // present, but I have choosen not to implement that.
+            // (warning: this code relies on the selection of quotation
+            // mark below)
+            $font = str_replace('\\', '\\5C ', $font);
+            $font = str_replace('"',  '\\22 ', $font);
+
             // complicated font, requires quoting
-
-            // armor single quotes and new lines
-            $font = str_replace("\\", "\\\\", $font);
-            $font = str_replace("'", "\\'", $font);
-            $final .= "'$font', ";
+            $final .= "\"$font\", "; // note that this will later get turned into &quot;
         }
         $final = rtrim($final, ', ');
         if ($final === '') return false;

=== modified file 'library/HTMLPurifier/AttrDef/CSS/URI.php'
--- library/HTMLPurifier/AttrDef/CSS/URI.php	2010-03-16 14:04:20 +0000
+++ library/HTMLPurifier/AttrDef/CSS/URI.php	2010-11-24 22:47:32 +0000
@@ -34,20 +34,16 @@
             $uri = substr($uri, 1, $new_length - 1);
         }
 
-        $keys   = array(  '(',   ')',   ',',   ' ',   '"',   "'");
-        $values = array('\\(', '\\)', '\\,', '\\ ', '\\"', "\\'");
-        $uri = str_replace($values, $keys, $uri);
+        $uri = $this->expandCSSEscape($uri);
 
         $result = parent::validate($uri, $config, $context);
 
         if ($result === false) return false;
 
-        // escape necessary characters according to CSS spec
-        // except for the comma, none of these should appear in the
-        // URI at all
-        $result = str_replace($keys, $values, $result);
+        // extra sanity check; should have been done by URI
+        $result = str_replace(array('"', "\\", "\n", "\x0c", "\r"), "", $result);
 
-        return "url($result)";
+        return "url(\"$result\")";
 
     }