[3.1.0] Implement DenyElementDecorator for imagecrash-protection against CSS width/height

- Misc doc changes
- Add missing inheritance for AttrDef_CSS decorators


git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1684 48356398-32a2-884e-a903-53898d9a118a

NEWS

@@ -13,6 +13,7 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
 - InterchangeBuilder now alphabetizes its lists
 - Validation error in configdoc output fixed
 - Iconv errors muted even with custom error handlers
+- Add protection against imagecrash attack with CSS height/width
 . Out-of-date documentation revised
 . UTF-8 encoding check optimization as suggested by Diego
 

TODO

@@ -15,7 +15,6 @@ UPCOMING RELEASE
 ----------------
 
 BUGS
- - Style attribute height/width limiting for images
  - Figure out what to do about target="" and name="", since they show up so often
 
 EXTERNAL

configdoc/generate.php

@@ -15,7 +15,7 @@ TODO:
 - add blurbs to ToC
 */
 
-if (version_compare(PHP_VERSION, '5.2.0', '<')) exit('PHP 5.2.0 or greater required.');
+if (version_compare(PHP_VERSION, '5.2', '<')) exit('PHP 5.2+ required.');
 error_reporting(E_ALL | E_STRICT);
 
 chdir(dirname(__FILE__));

configdoc/usage.xml

@@ -2,7 +2,7 @@
 <usage>
  <directive id="Core.CollectErrors">
   <file name="HTMLPurifier.php">
-   <line>131</line>
+   <line>129</line>
   </file>
   <file name="HTMLPurifier/Lexer.php">
    <line>93</line>
@@ -18,22 +18,22 @@
  </directive>
  <directive id="CSS.Proprietary">
   <file name="HTMLPurifier/CSSDefinition.php">
-   <line>201</line>
+   <line>202</line>
   </file>
  </directive>
  <directive id="CSS.AllowTricky">
   <file name="HTMLPurifier/CSSDefinition.php">
-   <line>205</line>
+   <line>206</line>
   </file>
  </directive>
  <directive id="CSS.AllowImportant">
   <file name="HTMLPurifier/CSSDefinition.php">
-   <line>209</line>
+   <line>210</line>
   </file>
  </directive>
  <directive id="CSS.AllowedProperties">
   <file name="HTMLPurifier/CSSDefinition.php">
-   <line>261</line>
+   <line>262</line>
   </file>
  </directive>
  <directive id="Cache.DefinitionImpl">
@@ -63,19 +63,19 @@
  </directive>
  <directive id="Core.Encoding">
   <file name="HTMLPurifier/Encoder.php">
-   <line>267</line>
+   <line>281</line>
-   <line>285</line>
+   <line>299</line>
   </file>
  </directive>
  <directive id="Test.ForceNoIconv">
   <file name="HTMLPurifier/Encoder.php">
-   <line>269</line>
+   <line>283</line>
-   <line>290</line>
+   <line>304</line>
   </file>
  </directive>
  <directive id="Core.EscapeNonASCIICharacters">
   <file name="HTMLPurifier/Encoder.php">
-   <line>287</line>
+   <line>301</line>
   </file>
  </directive>
  <directive id="Core.MaintainLineNumbers">

library/HTMLPurifier.includes.php

@@ -82,6 +82,7 @@ require 'HTMLPurifier/AttrDef/CSS/BackgroundPosition.php';
 require 'HTMLPurifier/AttrDef/CSS/Border.php';
 require 'HTMLPurifier/AttrDef/CSS/Color.php';
 require 'HTMLPurifier/AttrDef/CSS/Composite.php';
+require 'HTMLPurifier/AttrDef/CSS/DenyElementDecorator.php';
 require 'HTMLPurifier/AttrDef/CSS/Filter.php';
 require 'HTMLPurifier/AttrDef/CSS/Font.php';
 require 'HTMLPurifier/AttrDef/CSS/FontFamily.php';

library/HTMLPurifier.safe-includes.php

@@ -76,6 +76,7 @@ require_once $__dir . '/HTMLPurifier/AttrDef/CSS/BackgroundPosition.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/Border.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/Color.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/Composite.php';
+require_once $__dir . '/HTMLPurifier/AttrDef/CSS/DenyElementDecorator.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/Filter.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/Font.php';
 require_once $__dir . '/HTMLPurifier/AttrDef/CSS/FontFamily.php';

library/HTMLPurifier/AttrDef/CSS/DenyElementDecorator.php

@@ -0,0 +1,26 @@
+<?php
+
+/**
+ * Decorator which enables CSS properties to be disabled for specific elements.
+ */
+class HTMLPurifier_AttrDef_CSS_DenyElementDecorator extends HTMLPurifier_AttrDef
+{
+    protected $def, $element;
+    
+    /**
+     * @param $def Definition to wrap
+     * @param $element Element to deny
+     */
+    public function __construct($def, $element) {
+        $this->def = $def;
+        $this->element = $element;
+    }
+    /**
+     * Checks if CurrentToken is set and equal to $this->element
+     */
+    public function validate($string, $config, $context) {
+        $token = $context->get('CurrentToken', true);
+        if ($token && $token->name == $this->element) return false;
+        return $this->def->validate($string, $config, $context);
+    }
+}

library/HTMLPurifier/AttrDef/CSS/ImportantDecorator.php

@@ -3,7 +3,7 @@
 /**
  * Decorator which enables !important to be used in CSS values.
  */
-class HTMLPurifier_AttrDef_CSS_ImportantDecorator
+class HTMLPurifier_AttrDef_CSS_ImportantDecorator extends HTMLPurifier_AttrDef
 {
     protected $def, $allow;
     

library/HTMLPurifier/CSSDefinition.php

@@ -151,11 +151,12 @@ class HTMLPurifier_CSSDefinition extends HTMLPurifier_Definition
         
         $this->info['width'] =
         $this->info['height'] =
+        new HTMLPurifier_AttrDef_CSS_DenyElementDecorator(
         new HTMLPurifier_AttrDef_CSS_Composite(array(
             new HTMLPurifier_AttrDef_CSS_Length(true),
             new HTMLPurifier_AttrDef_CSS_Percentage(true),
             new HTMLPurifier_AttrDef_Enum(array('auto'))
-        ));
+        )), 'img');
         
         $this->info['text-decoration'] = new HTMLPurifier_AttrDef_CSS_TextDecoration();
         

library/HTMLPurifier/ConfigSchema/schema/Filter.ExtractStyleBlocks.txt

@@ -21,3 +21,17 @@ $styles = $purifier->context->get('StyleBlocks');
 foreach ($styles as $style) {
     echo '<style type="text/css">' . $style . "</style>\n";
 }]]></pre>
+<p>
+  <strong>Warning:</strong> It is possible for a user to mount an
+  imagecrash attack using this CSS. Counter-measures are difficult;
+  it is not simply enough to limit the range of CSS lengths (using
+  relative lengths with many nesting levels allows for large values
+  to be attained without actually specifying them in the stylesheet),
+  and the flexible nature of selectors makes it difficult to selectively
+  disable lengths on image tags (HTML Purifier, however, does disable
+  CSS width and height in inline styling). There are probably two effective
+  counter measures: an explicit width and height set to auto in all
+  images in your document (unlikely) or the disabling of width and
+  height (somewhat reasonable). Whether or not these measures should be
+  used is left to the reader.
+</p>

tests/HTMLPurifier/Strategy/ValidateAttributesTest.php

@@ -177,6 +177,13 @@ class HTMLPurifier_Strategy_ValidateAttributesTest extends
         );
     }
     
+    function testRemoveCSSWidthAndHeightOnImg() {
+        $this->assertResult(
+            '<img src="" alt="" style="width:10px;height:10px;border:1px solid #000;" />',
+            '<img src="" alt="" style="border:1px solid #000;" />'
+        );
+    }
+    
 }