Page MenuHomePhabricator
Files F10769788

T119158-master.patch
No One
Actions

Authored By
Bawolff
Nov 13 2017, 8:19 PM
Size
4 KB
Referenced Files
None
Subscribers
None

T119158-master.patch
View Options

From 9d54db356ec2956dda1668fd8cdb4a8fab72d572 Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Thu, 11 Feb 2016 17:08:03 -0500
Subject: [PATCH] SECURITY: Handle -{}- syntax in attributes safely
Previously, if one had an attribute with the contents
"-{}-foo-{}-", foo would get replaced by language converter as if
it wasn't in an attribute. This lead to an XSS attack.
This breaks doing manual conversions in url href's (or any
other attribute that goes through an escaping method
other than Sanitizer's). e.g. http://{sr-el:foo';sr-ec:bar}.com
won't work anymore. See also T87332
Bug: T119158
Change-Id: Idbc45cac12c309b0ccb4adeff6474fa527b48edb
---
languages/LanguageConverter.php | 39 +++++++++++++++++++++++++++------------
tests/parser/parserTests.txt | 14 ++++++++++++++
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/languages/LanguageConverter.php b/languages/LanguageConverter.php
index 00bc02d..f9610fa 100644
--- a/languages/LanguageConverter.php
+++ b/languages/LanguageConverter.php
@@ -376,9 +376,12 @@ class LanguageConverter {
$scriptfix = '<script[^>]*+>[^<]*+(?:(?:(?!<\/script>).)[^<]*+)*+<\/script>|';
// disable conversion of <pre> tags
$prefix = '<pre[^>]*+>[^<]*+(?:(?:(?!<\/pre>).)[^<]*+)*+<\/pre>|';
+ // The "|.*+)" at the end, is in case we missed some part of html syntax,
+ // we will fail securely (hopefully) by matching the rest of the string.
+ $htmlFullTag = '<(?:[^>=]*+(?>[^>=]*+=\s*+(?:"[^"]*"|\'[^\']*\'|[^\'">\s]*+))*+[^>=]*+>|.*+)|';
- $reg = '/' . $codefix . $scriptfix . $prefix .
- '<[^>]++>|&[a-zA-Z#][a-z0-9]++;' . $marker . $htmlfix . '|\004$/s';
+ $reg = '/' . $codefix . $scriptfix . $prefix . $htmlFullTag .
+ '&[a-zA-Z#][a-z0-9]++;' . $marker . $htmlfix . '|\004$/s';
$startPos = 0;
$sourceBlob = '';
$literalBlob = '';
@@ -658,29 +661,41 @@ class LanguageConverter {
$out = '';
$length = strlen( $text );
$shouldConvert = !$this->guessVariant( $text, $variant );
-
- while ( $startPos < $length ) {
- $pos = strpos( $text, '-{', $startPos );
-
- if ( $pos === false ) {
+ $continue = 1;
+
+ $noScript = '<script.*?>.*?<\/script>(*SKIP)(*FAIL)';
+ $noStyle = '<style.*?>.*?<\/style>(*SKIP)(*FAIL)';
+ $noHtml = '<(?:[^>=]*+(?>[^>=]*+=\s*+(?:"[^"]*"|\'[^\']*\'|[^\'">\s]*+))*+[^>=]*+>|.*+)(*SKIP)(*FAIL)';
+ while ( $startPos < $length && $continue ) {
+ $continue = preg_match(
+ // Only match -{ outside of html.
+ "/$noScript|$noStyle|$noHtml|-\{/",
+ $text,
+ $m,
+ PREG_OFFSET_CAPTURE,
+ $startPos
+ );
+
+ if ( !$continue ) {
// No more markup, append final segment
$fragment = substr( $text, $startPos );
$out .= $shouldConvert ? $this->autoConvert( $fragment, $variant ) : $fragment;
return $out;
}
- // Markup found
+ // Offset of the match of the regex pattern.
+ $pos = $m[0][1];
+
// Append initial segment
$fragment = substr( $text, $startPos, $pos - $startPos );
$out .= $shouldConvert ? $this->autoConvert( $fragment, $variant ) : $fragment;
-
- // Advance position
+ // -{ marker found, not in attribute
+ // Advance position up to -{ marker.
$startPos = $pos;
-
// Do recursive conversion
+ // Note: This passes $startPos by reference, and advances it.
$out .= $this->recursiveConvertRule( $text, $variant, $startPos, $depth + 1 );
}
-
return $out;
}
diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index cef935c..f9c6758 100644
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -18506,6 +18506,20 @@ all additional text is vanished
!! end
!! test
+Language converter glossary rules inside attributes (T119158)
+!! options
+language=sr variant=sr-el
+!! wikitext
+-{H|abc=>sr-el:" onload="alert(1)" data-foo="}-
+
+[[File:Foobar.jpg|alt=-{}-abc-{}-]]
+!! html
+<p>
+</p><p><a href="/wiki/%D0%94%D0%B0%D1%82%D0%BE%D1%82%D0%B5%D0%BA%D0%B0:Foobar.jpg" class="image"><img alt="&quot; onload=&quot;alert(1)&quot; data-foo=&quot;" src="http://example.com/images/3/3a/Foobar.jpg" width="1941" height="220"></a>
+</p>
+!! end
+
+!! test
Self closed html pairs (T7487)
!! wikitext
<center><font id="bug" />Centered text</center>
--
1.9.5 (Apple Git-50.3)

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5088088
Default Alt Text
T119158-master.patch (4 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