Page MenuHomePhabricator
Files F12218

0001-SECURITY-Always-quote-attribute-values-in-Html.patch
acl*security
Actions

Authored By
• bzimport
Nov 22 2014, 2:22 AM
Size
15 KB
Referenced Files
None
Subscribers
None

0001-SECURITY-Always-quote-attribute-values-in-Html.patch
View Options

From 1a395595dc3c96355b7152dc7afae9b2b2888b29 Mon Sep 17 00:00:00 2001
From: Kevin Israel <pleasestand@live.com>
Date: Sat, 16 Nov 2013 05:58:01 -0500
Subject: [PATCH] SECURITY: Always quote attribute values in Html
... even if $wgWellFormedXml is false.
Bug: 55548
Change-Id: Iaa83d6c0784cfdbbe0f3fc716088b70b6d1457e9
---
includes/Html.php | 23 +----
tests/phpunit/includes/HtmlTest.php | 200 ++++++++++++++++++------------------
2 files changed, 102 insertions(+), 121 deletions(-)
diff --git a/includes/Html.php b/includes/Html.php
index 932f753..7a6b44a 100644
--- a/includes/Html.php
+++ b/includes/Html.php
@@ -383,8 +383,7 @@ class Html {
* 'http://www.mediawiki.org/' ) becomes something like
* ' href="http://www.mediawiki.org"'. Again, this is like
* Xml::expandAttributes(), but it implements some HTML-specific logic.
- * For instance, it will omit quotation marks if $wgWellFormedXml is false,
- * and will treat boolean attributes specially.
+ * For instance, it will treat boolean attributes specially.
*
* Attributes that should contain space-separated lists (such as 'class') array
* values are allowed as well, which will automagically be normalized
@@ -502,24 +501,6 @@ class Html {
$value = implode( ' ', array_unique( $value ) );
}
- // See the "Attributes" section in the HTML syntax part of HTML5,
- // 9.1.2.3 as of 2009-08-10. Most attributes can have quotation
- // marks omitted, but not all. (Although a literal " is not
- // permitted, we don't check for that, since it will be escaped
- // anyway.)
- #
- // See also research done on further characters that need to be
- // escaped: http://code.google.com/p/html5lib/issues/detail?id=93
- $badChars = "\\x00- '=<>`/\x{00a0}\x{1680}\x{180e}\x{180F}\x{2000}\x{2001}"
- . "\x{2002}\x{2003}\x{2004}\x{2005}\x{2006}\x{2007}\x{2008}\x{2009}"
- . "\x{200A}\x{2028}\x{2029}\x{202F}\x{205F}\x{3000}";
- if ( $wgWellFormedXml || $value === ''
- || preg_match( "![$badChars]!u", $value ) ) {
- $quote = '"';
- } else {
- $quote = '';
- }
-
if ( in_array( $key, self::$boolAttribs ) ) {
// In HTML5, we can leave the value empty. If we don't need
// well-formed XML, we can omit the = entirely.
@@ -552,7 +533,7 @@ class Html {
// @todo FIXME: Is this really true?
$map['<'] = '&lt;';
}
- $ret .= " $key=$quote" . strtr( $value, $map ) . $quote;
+ $ret .= " $key=\"" . strtr( $value, $map ) . '"';
}
}
return $ret;
diff --git a/tests/phpunit/includes/HtmlTest.php b/tests/phpunit/includes/HtmlTest.php
index 21372a0..15b7291 100644
--- a/tests/phpunit/includes/HtmlTest.php
+++ b/tests/phpunit/includes/HtmlTest.php
@@ -163,22 +163,22 @@ class HtmlTest extends MediaWikiTestCase {
$this->assertEquals(
' empty_string=""',
Html::expandAttributes( array( 'empty_string' => '' ) ),
- 'Empty string is always quoted'
+ 'Attribute values are always quoted: Empty string'
);
$this->assertEquals(
- ' key=value',
+ ' key="value"',
Html::expandAttributes( array( 'key' => 'value' ) ),
- 'Simple string value needs no quotes'
+ 'Attribute values are always quoted: Simple string'
);
$this->assertEquals(
- ' one=1',
+ ' one="1"',
Html::expandAttributes( array( 'one' => 1 ) ),
- 'Number 1 value needs no quotes'
+ 'Attribute values are always quoted: Number 1'
);
$this->assertEquals(
- ' zero=0',
+ ' zero="0"',
Html::expandAttributes( array( 'zero' => 0 ) ),
- 'Number 0 value needs no quotes'
+ 'Attribute values are always quoted: Number 0'
);
$this->setMwGlobals( 'wgWellFormedXml', true );
@@ -304,48 +304,48 @@ class HtmlTest extends MediaWikiTestCase {
*/
public function testNamespaceSelector() {
$this->assertEquals(
- '<select id=namespace name=namespace>' . "\n" .
- '<option value=0>(Main)</option>' . "\n" .
- '<option value=1>Talk</option>' . "\n" .
- '<option value=2>User</option>' . "\n" .
- '<option value=3>User talk</option>' . "\n" .
- '<option value=4>MyWiki</option>' . "\n" .
- '<option value=5>MyWiki Talk</option>' . "\n" .
- '<option value=6>File</option>' . "\n" .
- '<option value=7>File talk</option>' . "\n" .
- '<option value=8>MediaWiki</option>' . "\n" .
- '<option value=9>MediaWiki talk</option>' . "\n" .
- '<option value=10>Template</option>' . "\n" .
- '<option value=11>Template talk</option>' . "\n" .
- '<option value=14>Category</option>' . "\n" .
- '<option value=15>Category talk</option>' . "\n" .
- '<option value=100>Custom</option>' . "\n" .
- '<option value=101>Custom talk</option>' . "\n" .
+ '<select id="namespace" name="namespace">' . "\n" .
+ '<option value="0">(Main)</option>' . "\n" .
+ '<option value="1">Talk</option>' . "\n" .
+ '<option value="2">User</option>' . "\n" .
+ '<option value="3">User talk</option>' . "\n" .
+ '<option value="4">MyWiki</option>' . "\n" .
+ '<option value="5">MyWiki Talk</option>' . "\n" .
+ '<option value="6">File</option>' . "\n" .
+ '<option value="7">File talk</option>' . "\n" .
+ '<option value="8">MediaWiki</option>' . "\n" .
+ '<option value="9">MediaWiki talk</option>' . "\n" .
+ '<option value="10">Template</option>' . "\n" .
+ '<option value="11">Template talk</option>' . "\n" .
+ '<option value="14">Category</option>' . "\n" .
+ '<option value="15">Category talk</option>' . "\n" .
+ '<option value="100">Custom</option>' . "\n" .
+ '<option value="101">Custom talk</option>' . "\n" .
'</select>',
Html::namespaceSelector(),
'Basic namespace selector without custom options'
);
$this->assertEquals(
- '<label for=mw-test-namespace>Select a namespace:</label>&#160;' .
- '<select id=mw-test-namespace name=wpNamespace>' . "\n" .
- '<option value=all>all</option>' . "\n" .
- '<option value=0>(Main)</option>' . "\n" .
- '<option value=1>Talk</option>' . "\n" .
- '<option value=2 selected>User</option>' . "\n" .
- '<option value=3>User talk</option>' . "\n" .
- '<option value=4>MyWiki</option>' . "\n" .
- '<option value=5>MyWiki Talk</option>' . "\n" .
- '<option value=6>File</option>' . "\n" .
- '<option value=7>File talk</option>' . "\n" .
- '<option value=8>MediaWiki</option>' . "\n" .
- '<option value=9>MediaWiki talk</option>' . "\n" .
- '<option value=10>Template</option>' . "\n" .
- '<option value=11>Template talk</option>' . "\n" .
- '<option value=14>Category</option>' . "\n" .
- '<option value=15>Category talk</option>' . "\n" .
- '<option value=100>Custom</option>' . "\n" .
- '<option value=101>Custom talk</option>' . "\n" .
+ '<label for="mw-test-namespace">Select a namespace:</label>&#160;' .
+ '<select id="mw-test-namespace" name="wpNamespace">' . "\n" .
+ '<option value="all">all</option>' . "\n" .
+ '<option value="0">(Main)</option>' . "\n" .
+ '<option value="1">Talk</option>' . "\n" .
+ '<option value="2" selected>User</option>' . "\n" .
+ '<option value="3">User talk</option>' . "\n" .
+ '<option value="4">MyWiki</option>' . "\n" .
+ '<option value="5">MyWiki Talk</option>' . "\n" .
+ '<option value="6">File</option>' . "\n" .
+ '<option value="7">File talk</option>' . "\n" .
+ '<option value="8">MediaWiki</option>' . "\n" .
+ '<option value="9">MediaWiki talk</option>' . "\n" .
+ '<option value="10">Template</option>' . "\n" .
+ '<option value="11">Template talk</option>' . "\n" .
+ '<option value="14">Category</option>' . "\n" .
+ '<option value="15">Category talk</option>' . "\n" .
+ '<option value="100">Custom</option>' . "\n" .
+ '<option value="101">Custom talk</option>' . "\n" .
'</select>',
Html::namespaceSelector(
array( 'selected' => '2', 'all' => 'all', 'label' => 'Select a namespace:' ),
@@ -355,24 +355,24 @@ class HtmlTest extends MediaWikiTestCase {
);
$this->assertEquals(
- '<label for=namespace>Select a namespace:</label>&#160;' .
- '<select id=namespace name=namespace>' . "\n" .
- '<option value=0>(Main)</option>' . "\n" .
- '<option value=1>Talk</option>' . "\n" .
- '<option value=2>User</option>' . "\n" .
- '<option value=3>User talk</option>' . "\n" .
- '<option value=4>MyWiki</option>' . "\n" .
- '<option value=5>MyWiki Talk</option>' . "\n" .
- '<option value=6>File</option>' . "\n" .
- '<option value=7>File talk</option>' . "\n" .
- '<option value=8>MediaWiki</option>' . "\n" .
- '<option value=9>MediaWiki talk</option>' . "\n" .
- '<option value=10>Template</option>' . "\n" .
- '<option value=11>Template talk</option>' . "\n" .
- '<option value=14>Category</option>' . "\n" .
- '<option value=15>Category talk</option>' . "\n" .
- '<option value=100>Custom</option>' . "\n" .
- '<option value=101>Custom talk</option>' . "\n" .
+ '<label for="namespace">Select a namespace:</label>&#160;' .
+ '<select id="namespace" name="namespace">' . "\n" .
+ '<option value="0">(Main)</option>' . "\n" .
+ '<option value="1">Talk</option>' . "\n" .
+ '<option value="2">User</option>' . "\n" .
+ '<option value="3">User talk</option>' . "\n" .
+ '<option value="4">MyWiki</option>' . "\n" .
+ '<option value="5">MyWiki Talk</option>' . "\n" .
+ '<option value="6">File</option>' . "\n" .
+ '<option value="7">File talk</option>' . "\n" .
+ '<option value="8">MediaWiki</option>' . "\n" .
+ '<option value="9">MediaWiki talk</option>' . "\n" .
+ '<option value="10">Template</option>' . "\n" .
+ '<option value="11">Template talk</option>' . "\n" .
+ '<option value="14">Category</option>' . "\n" .
+ '<option value="15">Category talk</option>' . "\n" .
+ '<option value="100">Custom</option>' . "\n" .
+ '<option value="101">Custom talk</option>' . "\n" .
'</select>',
Html::namespaceSelector(
array( 'label' => 'Select a namespace:' )
@@ -383,18 +383,18 @@ class HtmlTest extends MediaWikiTestCase {
public function testCanFilterOutNamespaces() {
$this->assertEquals(
- '<select id=namespace name=namespace>' . "\n" .
- '<option value=2>User</option>' . "\n" .
- '<option value=4>MyWiki</option>' . "\n" .
- '<option value=5>MyWiki Talk</option>' . "\n" .
- '<option value=6>File</option>' . "\n" .
- '<option value=7>File talk</option>' . "\n" .
- '<option value=8>MediaWiki</option>' . "\n" .
- '<option value=9>MediaWiki talk</option>' . "\n" .
- '<option value=10>Template</option>' . "\n" .
- '<option value=11>Template talk</option>' . "\n" .
- '<option value=14>Category</option>' . "\n" .
- '<option value=15>Category talk</option>' . "\n" .
+ '<select id="namespace" name="namespace">' . "\n" .
+ '<option value="2">User</option>' . "\n" .
+ '<option value="4">MyWiki</option>' . "\n" .
+ '<option value="5">MyWiki Talk</option>' . "\n" .
+ '<option value="6">File</option>' . "\n" .
+ '<option value="7">File talk</option>' . "\n" .
+ '<option value="8">MediaWiki</option>' . "\n" .
+ '<option value="9">MediaWiki talk</option>' . "\n" .
+ '<option value="10">Template</option>' . "\n" .
+ '<option value="11">Template talk</option>' . "\n" .
+ '<option value="14">Category</option>' . "\n" .
+ '<option value="15">Category talk</option>' . "\n" .
'</select>',
Html::namespaceSelector(
array( 'exclude' => array( 0, 1, 3, 100, 101 ) )
@@ -405,23 +405,23 @@ class HtmlTest extends MediaWikiTestCase {
public function testCanDisableANamespaces() {
$this->assertEquals(
- '<select id=namespace name=namespace>' . "\n" .
- '<option disabled value=0>(Main)</option>' . "\n" .
- '<option disabled value=1>Talk</option>' . "\n" .
- '<option disabled value=2>User</option>' . "\n" .
- '<option disabled value=3>User talk</option>' . "\n" .
- '<option disabled value=4>MyWiki</option>' . "\n" .
- '<option value=5>MyWiki Talk</option>' . "\n" .
- '<option value=6>File</option>' . "\n" .
- '<option value=7>File talk</option>' . "\n" .
- '<option value=8>MediaWiki</option>' . "\n" .
- '<option value=9>MediaWiki talk</option>' . "\n" .
- '<option value=10>Template</option>' . "\n" .
- '<option value=11>Template talk</option>' . "\n" .
- '<option value=14>Category</option>' . "\n" .
- '<option value=15>Category talk</option>' . "\n" .
- '<option value=100>Custom</option>' . "\n" .
- '<option value=101>Custom talk</option>' . "\n" .
+ '<select id="namespace" name="namespace">' . "\n" .
+ '<option disabled value="0">(Main)</option>' . "\n" .
+ '<option disabled value="1">Talk</option>' . "\n" .
+ '<option disabled value="2">User</option>' . "\n" .
+ '<option disabled value="3">User talk</option>' . "\n" .
+ '<option disabled value="4">MyWiki</option>' . "\n" .
+ '<option value="5">MyWiki Talk</option>' . "\n" .
+ '<option value="6">File</option>' . "\n" .
+ '<option value="7">File talk</option>' . "\n" .
+ '<option value="8">MediaWiki</option>' . "\n" .
+ '<option value="9">MediaWiki talk</option>' . "\n" .
+ '<option value="10">Template</option>' . "\n" .
+ '<option value="11">Template talk</option>' . "\n" .
+ '<option value="14">Category</option>' . "\n" .
+ '<option value="15">Category talk</option>' . "\n" .
+ '<option value="100">Custom</option>' . "\n" .
+ '<option value="101">Custom talk</option>' . "\n" .
'</select>',
Html::namespaceSelector( array(
'disable' => array( 0, 1, 2, 3, 4 )
@@ -436,7 +436,7 @@ class HtmlTest extends MediaWikiTestCase {
*/
public function testHtmlElementAcceptsNewHtml5TypesInHtml5Mode( $HTML5InputType ) {
$this->assertEquals(
- '<input type=' . $HTML5InputType . '>',
+ '<input type="' . $HTML5InputType . '">',
Html::element( 'input', array( 'type' => $HTML5InputType ) ),
'In HTML5, HTML::element() should accept type="' . $HTML5InputType . '"'
);
@@ -490,10 +490,10 @@ class HtmlTest extends MediaWikiTestCase {
'area', array( 'shape' => 'rect' )
);
- $cases[] = array( '<button type=submit></button>',
+ $cases[] = array( '<button type="submit"></button>',
'button', array( 'formaction' => 'GET' )
);
- $cases[] = array( '<button type=submit></button>',
+ $cases[] = array( '<button type="submit"></button>',
'button', array( 'formenctype' => 'application/x-www-form-urlencoded' )
);
@@ -567,28 +567,28 @@ class HtmlTest extends MediaWikiTestCase {
);
# <input> specific handling
- $cases[] = array( '<input type=checkbox>',
+ $cases[] = array( '<input type="checkbox">',
'input', array( 'type' => 'checkbox', 'value' => 'on' ),
'Default value "on" is stripped of checkboxes',
);
- $cases[] = array( '<input type=radio>',
+ $cases[] = array( '<input type="radio">',
'input', array( 'type' => 'radio', 'value' => 'on' ),
'Default value "on" is stripped of radio buttons',
);
- $cases[] = array( '<input type=submit value=Submit>',
+ $cases[] = array( '<input type="submit" value="Submit">',
'input', array( 'type' => 'submit', 'value' => 'Submit' ),
'Default value "Submit" is kept on submit buttons (for possible l10n issues)',
);
- $cases[] = array( '<input type=color>',
+ $cases[] = array( '<input type="color">',
'input', array( 'type' => 'color', 'value' => '' ),
);
- $cases[] = array( '<input type=range>',
+ $cases[] = array( '<input type="range">',
'input', array( 'type' => 'range', 'value' => '' ),
);
# <button> specific handling
# see remarks on http://msdn.microsoft.com/en-us/library/ie/ms535211%28v=vs.85%29.aspx
- $cases[] = array( '<button type=submit></button>',
+ $cases[] = array( '<button type="submit"></button>',
'button', array( 'type' => 'submit' ),
'According to standard the default type is "submit". Depending on compatibility mode IE might use "button", instead.',
);
@@ -644,7 +644,7 @@ class HtmlTest extends MediaWikiTestCase {
'Blacklist form validation attributes.'
);
$this->assertEquals(
- ' step=any',
+ ' step="any"',
Html::expandAttributes( array( 'min' => 1, 'max' => 100, 'pattern' => 'abc', 'required' => true, 'step' => 'any' ) ),
'Allow special case "step=any".'
);
--
1.8.4.2

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
11675
Default Alt Text
0001-SECURITY-Always-quote-attribute-values-in-Html.patch (15 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits