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

Proposed by Artur Rona
Status: Needs review
Proposed branch: lp:~ari-tczew/ubuntu/lucid/php-htmlpurifier/security
Merge into: lp:ubuntu/lucid/php-htmlpurifier
Diff against target: 178 lines (+69/-44)
5 files modified
debian/changelog (+10/-0)
debian/control (+2/-1)
library/HTMLPurifier/AttrDef.php (+36/-0)
library/HTMLPurifier/AttrDef/CSS/FontFamily.php (+17/-35)
library/HTMLPurifier/AttrDef/CSS/URI.php (+4/-8)
To merge this branch: bzr merge lp:~ari-tczew/ubuntu/lucid/php-htmlpurifier/security
Reviewer Review Type Date Requested Status
Artur Rona (community) Needs Resubmitting
Jamie Strandboge Needs Fixing
Review via email: mp+29504@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

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

review: Needs Fixing
7. By Artur Rona

Remove done changes due to not correctly with packaging policy.

8. By Artur Rona

Patch files directly.

Revision history for this message
Artur Rona (ari-tczew) wrote :

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

Patch files directly.

7. By Artur Rona

Remove done changes due to not correctly with packaging policy.

6. By Artur Rona

* 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1php-htmlpurifier (4.0.0+dfsg1-1ubuntu0.1) lucid-security; urgency=low
2
3 * SECURITY UPDATE (LP: #582576).
4 * A vulnerability has been reported in HTML Purifier, which can be
5 exploited by malicious people to conduct cross-site scripting
6 attacks.
7 * CVE-2010-2479
8
9 -- Artur Rona <ari-tczew@ubuntu.com> Wed, 24 Nov 2010 22:36:10 +0100
10
1php-htmlpurifier (4.0.0+dfsg1-1) unstable; urgency=low11php-htmlpurifier (4.0.0+dfsg1-1) unstable; urgency=low
212
3 * Take original upstream tarball, removing non-DFSG-free XHTML13 * Take original upstream tarball, removing non-DFSG-free XHTML
414
=== 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 @@
1Source: php-htmlpurifier1Source: php-htmlpurifier
2Section: php2Section: php
3Priority: optional3Priority: optional
4Maintainer: Christian Bayle <bayle@debian.org>4Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
5XSBC-Original-Maintainer: Christian Bayle <bayle@debian.org>
5Uploaders: Thorsten Glaser <tg@mirbsd.de>, Roland Mas <lolando@debian.org>6Uploaders: Thorsten Glaser <tg@mirbsd.de>, Roland Mas <lolando@debian.org>
6Homepage: http://htmlpurifier.org/7Homepage: http://htmlpurifier.org/
7Build-Depends: debhelper (>= 5)8Build-Depends: debhelper (>= 5)
89
=== 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 @@
82 return preg_replace('/rgb\((\d+)\s*,\s*(\d+)\s*,\s*(\d+)\)/', 'rgb(\1,\2,\3)', $string);82 return preg_replace('/rgb\((\d+)\s*,\s*(\d+)\s*,\s*(\d+)\)/', 'rgb(\1,\2,\3)', $string);
83 }83 }
8484
85 /**
86 * Parses a possibly escaped CSS string and returns the "pure"
87 * version of it.
88 */
89 protected function expandCSSEscape($string) {
90 // flexibly parse it
91 $ret = '';
92 for ($i = 0, $c = strlen($string); $i < $c; $i++) {
93 if ($string[$i] === '\\') {
94 $i++;
95 if ($i >= $c) {
96 $ret .= '\\';
97 break;
98 }
99 if (ctype_xdigit($string[$i])) {
100 $code = $string[$i];
101 for ($a = 1, $i++; $i < $c && $a < 6; $i++, $a++) {
102 if (!ctype_xdigit($string[$i])) break;
103 $code .= $string[$i];
104 }
105 // We have to be extremely careful when adding
106 // new characters, to make sure we're not breaking
107 // the encoding.
108 $char = HTMLPurifier_Encoder::unichr(hexdec($code));
109 if (HTMLPurifier_Encoder::cleanUTF8($char) === '') continue;
110 $ret .= $char;
111 if ($i < $c && trim($string[$i]) !== '') $i--;
112 continue;
113 }
114 if ($string[$i] === "\n") continue;
115 }
116 $ret .= $string[$i];
117 }
118 return $ret;
119 }
120
85}121}
86122
87// vim: et sw=4 sts=4123// vim: et sw=4 sts=4
88124
=== 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 @@
34 $quote = $font[0];34 $quote = $font[0];
35 if ($font[$length - 1] !== $quote) continue;35 if ($font[$length - 1] !== $quote) continue;
36 $font = substr($font, 1, $length - 2);36 $font = substr($font, 1, $length - 2);
37
38 $new_font = '';
39 for ($i = 0, $c = strlen($font); $i < $c; $i++) {
40 if ($font[$i] === '\\') {
41 $i++;
42 if ($i >= $c) {
43 $new_font .= '\\';
44 break;
45 }
46 if (ctype_xdigit($font[$i])) {
47 $code = $font[$i];
48 for ($a = 1, $i++; $i < $c && $a < 6; $i++, $a++) {
49 if (!ctype_xdigit($font[$i])) break;
50 $code .= $font[$i];
51 }
52 // We have to be extremely careful when adding
53 // new characters, to make sure we're not breaking
54 // the encoding.
55 $char = HTMLPurifier_Encoder::unichr(hexdec($code));
56 if (HTMLPurifier_Encoder::cleanUTF8($char) === '') continue;
57 $new_font .= $char;
58 if ($i < $c && trim($font[$i]) !== '') $i--;
59 continue;
60 }
61 if ($font[$i] === "\n") continue;
62 }
63 $new_font .= $font[$i];
64 }
65
66 $font = $new_font;
67 }37 }
38
39 $font = $this->expandCSSEscape($font);
40
68 // $font is a pure representation of the font name41 // $font is a pure representation of the font name
6942
70 if (ctype_alnum($font) && $font !== '') {43 if (ctype_alnum($font) && $font !== '') {
@@ -73,12 +46,21 @@
73 continue;46 continue;
74 }47 }
7548
49 // bugger out on whitespace. form feed (0C) really
50 // shouldn't show up regardless
51 $font = str_replace(array("\n", "\t", "\r", "\x0C"), ' ', $font);
52
53 // These ugly transforms don't pose a security
54 // risk (as \\ and \" might). We could try to be clever and
55 // use single-quote wrapping when there is a double quote
56 // present, but I have choosen not to implement that.
57 // (warning: this code relies on the selection of quotation
58 // mark below)
59 $font = str_replace('\\', '\\5C ', $font);
60 $font = str_replace('"', '\\22 ', $font);
61
76 // complicated font, requires quoting62 // complicated font, requires quoting
7763 $final .= "\"$font\", "; // note that this will later get turned into &quot;
78 // armor single quotes and new lines
79 $font = str_replace("\\", "\\\\", $font);
80 $font = str_replace("'", "\\'", $font);
81 $final .= "'$font', ";
82 }64 }
83 $final = rtrim($final, ', ');65 $final = rtrim($final, ', ');
84 if ($final === '') return false;66 if ($final === '') return false;
8567
=== 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 @@
34 $uri = substr($uri, 1, $new_length - 1);34 $uri = substr($uri, 1, $new_length - 1);
35 }35 }
3636
37 $keys = array( '(', ')', ',', ' ', '"', "'");37 $uri = $this->expandCSSEscape($uri);
38 $values = array('\\(', '\\)', '\\,', '\\ ', '\\"', "\\'");
39 $uri = str_replace($values, $keys, $uri);
4038
41 $result = parent::validate($uri, $config, $context);39 $result = parent::validate($uri, $config, $context);
4240
43 if ($result === false) return false;41 if ($result === false) return false;
4442
45 // escape necessary characters according to CSS spec43 // extra sanity check; should have been done by URI
46 // except for the comma, none of these should appear in the44 $result = str_replace(array('"', "\\", "\n", "\x0c", "\r"), "", $result);
47 // URI at all
48 $result = str_replace($keys, $values, $result);
4945
50 return "url($result)";46 return "url(\"$result\")";
5147
52 }48 }
5349

Subscribers

People subscribed via source and target branches

to all changes: