Strip markers get substituted for general html, which means the
substitution text general does not escape quote characters. If
someone can convince MW to put a strip marker in an attribute,
you can get around escaping requirements that way. This patch
adds the characters `"' to the strip marker text. At least one
of these characters should be escaped inside attributes (regardless
of what quote character you use for attributes), thus normal html
escaping will deactivate the strip markers, preventing the
vulnrability.
This will break any extension that escapes input with htmlspecialchars,
to add to html/half parsed html output, but assumes that strip markers
are unmangled. I don't think its very common to do this. The primary
example I found was some core usages of Xml::escapeTagsOnly(). (And
even in that case, it only affected the corner case of being called
via {{#tag:..}})
Based on MatmaRex's suggestion.
Change-Id: If887065e12026530f36e5f35dd7ab0831d313561
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
$content = StringUtils::delimiterReplace( '<nowiki>', '</nowiki>', '$1', $text, 'i' );
$attribs = Sanitizer::validateTagAttributes( $attribs, 'pre' );
$content = StringUtils::delimiterReplace( '<nowiki>', '</nowiki>', '$1', $text, 'i' );
$attribs = Sanitizer::validateTagAttributes( $attribs, 'pre' );
- return Xml::openElement( 'pre', $attribs ) .
- Xml::escapeTagsOnly( $content ) .
- '</pre>';
+ // We need to let both '"' and '&' through,
+ // for strip markers and entities respectively.
+ $content = str_replace(
+ array( '>', '<' ),
+ array( '>', '<' ),
+ $content
+ );
+ return Html::rawElement( 'pre', $attribs, $content );
* @return array
*/
public static function nowiki( $content, $attributes, $parser ) {
* @return array
*/
public static function nowiki( $content, $attributes, $parser ) {
- $content = strtr( $content, [ '-{' => '-{', '}-' => '}-' ] );
- return [ Xml::escapeTagsOnly( $content ), 'markerType' => 'nowiki' ];
+ $content = strtr( $content, array(
+ // lang converter
+ '-{' => '-{',
+ '}-' => '}-',
+ // html tags
+ '<' => '<',
+ '>' => '>'
+ // Note: Both '"' and '&' are not converted.
+ // This allows strip markers and entities through.
+ ) );
+ return array( $content, 'markerType' => 'nowiki' );
*
* Must not consist of all title characters, or else it will change
* the behavior of <nowiki> in a link.
*
* Must not consist of all title characters, or else it will change
* the behavior of <nowiki> in a link.
+ *
+ * Must have a character that needs escaping in attributes, otherwise
+ * someone could put a strip marker in an attribute, to get around
+ * escaping quote marks, and break out of the attribute. Thus we add
+ * `'".
- const MARKER_SUFFIX = "-QINU\x7f";
- const MARKER_PREFIX = "\x7fUNIQ-";
+ const MARKER_SUFFIX = "-QINU`\"'\x7f";
+ const MARKER_PREFIX = "\x7f'\"`UNIQ-";
# Markers used for wrapping the table of contents
const TOC_START = '<mw:toc>';
# Markers used for wrapping the table of contents
const TOC_START = '<mw:toc>';
+!! test
+<nowiki> inside of #tag:pre
+!! wikitext
+{{#tag:pre|Foo <nowiki>→bar</nowiki>}}
+!! html
+<pre>Foo →bar</pre>
+
+!! end
+
!! test
<nowiki> and <pre> preference (first one wins)
!! wikitext
!! test
<nowiki> and <pre> preference (first one wins)
!! wikitext