r85858 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r85857‎ | r85858 | r85859 >
Date:02:11, 12 April 2011
Author:tstarling
Status:ok
Tags:
Comment:
MFT r85856: Fix for bug 28450: escaped CSS comments
Modified paths:
  • /branches/REL1_17/phase3/includes/Sanitizer.php (modified) (history)

Diff [purge]

Index: branches/REL1_17/phase3/includes/Sanitizer.php
@@ -735,28 +735,34 @@
736736
737737 /**
738738 * Pick apart some CSS and check it for forbidden or unsafe structures.
739 - * Returns a sanitized string, or false if it was just too evil.
 739+ * Returns a sanitized string. This sanitized string will have
 740+ * character references and escape sequences decoded, and comments
 741+ * stripped. If the input is just too evil, only a comment complaining
 742+ * about evilness will be returned.
740743 *
741744 * Currently URL references, 'expression', 'tps' are forbidden.
742745 *
 746+ * NOTE: Despite the fact that character references are decoded, the
 747+ * returned string may contain character references given certain
 748+ * clever input strings. These character references must
 749+ * be escaped before the return value is embedded in HTML.
 750+ *
743751 * @param $value String
744 - * @return Mixed
 752+ * @return String
745753 */
746754 static function checkCss( $value ) {
 755+ // Decode character references like {
747756 $value = Sanitizer::decodeCharReferences( $value );
748757
749 - // Remove any comments; IE gets token splitting wrong
750 - $value = StringUtils::delimiterReplace( '/*', '*/', ' ', $value );
751 -
752 - // Remove anything after a comment-start token, to guard against
753 - // incorrect client implementations.
754 - $commentPos = strpos( $value, '/*' );
755 - if ( $commentPos !== false ) {
756 - $value = substr( $value, 0, $commentPos );
757 - }
758 -
759758 // Decode escape sequences and line continuation
760759 // See the grammar in the CSS 2 spec, appendix D.
 760+ // This has to be done AFTER decoding character references.
 761+ // This means it isn't possible for this function to return
 762+ // unsanitized escape sequences. It is possible to manufacture
 763+ // input that contains character references that decode to
 764+ // escape sequences that decode to character references, but
 765+ // it's OK for the return value to contain character references
 766+ // because the caller is supposed to escape those anyway.
761767 static $decodeRegex;
762768 if ( !$decodeRegex ) {
763769 $space = '[\\x20\\t\\r\\n\\f]';
@@ -772,7 +778,22 @@
773779 }
774780 $value = preg_replace_callback( $decodeRegex,
775781 array( __CLASS__, 'cssDecodeCallback' ), $value );
 782+
 783+ // Remove any comments; IE gets token splitting wrong
 784+ // This must be done AFTER decoding character references and
 785+ // escape sequences, because those steps can introduce comments
 786+ // This step cannot introduce character references or escape
 787+ // sequences, because it replaces comments with spaces rather
 788+ // than removing them completely.
 789+ $value = StringUtils::delimiterReplace( '/*', '*/', ' ', $value );
776790
 791+ // Remove anything after a comment-start token, to guard against
 792+ // incorrect client implementations.
 793+ $commentPos = strpos( $value, '/*' );
 794+ if ( $commentPos !== false ) {
 795+ $value = substr( $value, 0, $commentPos );
 796+ }
 797+
777798 // Reject problematic keywords and control characters
778799 if ( preg_match( '/[\000-\010\016-\037\177]/', $value ) ) {
779800 return '/* invalid control char */';

Follow-up revisions

RevisionCommit summaryAuthorDate
r85859MFT r85856: Fix for bug 28450: escaped CSS commentststarling02:11, 12 April 2011
r85860Release notes for r85858.tstarling03:07, 12 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r85856Fix for bug 28450: escaped CSS commentststarling02:10, 12 April 2011
r85857MFT r85856: Fix for bug 28450: escaped CSS commentststarling02:10, 12 April 2011

Status & tagging log