Merge branch into major

CHANGES.md

@@ -3,6 +3,14 @@
 
 ## CKEditor 4.11
 
+**Security Updates:**
+
+* Fixed XSS vulnerability in the HTML parser reported by [maxarr](https://hackerone.com/maxarr).
+
+	Issue summary: It was possible to execute XSS inside CKEditor after persuading the victim to: (i) switch CKEditor to source mode, then (ii) paste a specially crafted HTML code, prepared by the attacker, into the opened CKEditor source area, and (iii) switch back to WYSIWYG mode.
+
+**An upgrade is highly recommended!**
+
 New Features:
 
 * [#2062](https://github.com/ckeditor/ckeditor-dev/pull/2062): Added the emoji dropdown that allows the user to choose the emoji from the toolbar and search for them using keywords.

core/htmldataprocessor.js

@@ -52,6 +52,10 @@
 				data = evtData.dataValue,
 				fixBodyTag;
 
+			// Before we start protecting markup, make sure there's no externally injected <cke:encoded> elements. Only
+			// HTML processor can use this tag, any external injections should discarded.
+			data = data.replace( reservedElementsRegex, '' );
+
 			// The source data is already HTML, but we need to clean
 			// it up and apply the filter.
 			data = protectSource( data, editor );
@@ -797,7 +801,8 @@
 		// Note: we use lazy star '*?' to prevent eating everything up to the last occurrence of </style> or </textarea>.
 	var protectElementsRegex = /(?:<style(?=[ >])[^>]*>[\s\S]*?<\/style>)|(?:<(:?link|meta|base)[^>]*>)/gi,
 		protectTextareaRegex = /(<textarea(?=[ >])[^>]*>)([\s\S]*?)(?:<\/textarea>)/gi,
-		encodedElementsRegex = /<cke:encoded>([^<]*)<\/cke:encoded>/gi;
+		encodedElementsRegex = /<cke:encoded>([^<]*)<\/cke:encoded>/gi,
+		reservedElementsRegex = createReservedElementsRegex();
 
 		// Element name should be followed by space or closing angle bracket '>' to not protect custom tags (#988).
 	var protectElementNamesRegex = /(<\/?)((?:object|embed|param|html|body|head|title)([\s][^>]*)?>)/gi,
@@ -860,6 +865,72 @@
 		} );
 	}
 
+	// Produces regex matching reserved `cke:encoded` element for valid HTML symbol codes.
+	// Matches `cke:encoded` element in hexadecimal, HTML-code, or HTML-entity.
+	function createReservedElementsRegex() {
+		return new RegExp( '(' +
+			// Create closed element regex i.e `<cke:encoded>xxx</cke:encoded>`.
+			createEncodedRegex( '<cke:encoded>' ) +
+			'(.*?)' +
+			createEncodedRegex( '</cke:encoded>' ) +
+			')|(' +
+			// Create unclosed element regex i.e `<cke:encoded>xxx` or `xxx</cke:encoded>` to make sure that
+			// element won't be closed by HTML parser and matched by `unprotectElements` function.
+			createEncodedRegex( '<' ) +
+			createEncodedRegex( '/' ) + '?' +
+			createEncodedRegex( 'cke:encoded>' ) +
+			')', 'gi' );
+	}
+
+	function createEncodedRegex( str ) {
+		return CKEDITOR.tools.array.reduce( str.split( '' ), function( cur, character ) {
+			// Produce case insensitive regex. `i` flag is not enough thus code entities differs
+			// depending on case sensitivity.
+			var lowerCase = character.toLowerCase(),
+				upperCase = character.toUpperCase(),
+				regex = createCharacterEncodedRegex( lowerCase );
+
+			if ( lowerCase !== upperCase ) {
+				regex += '|' + createCharacterEncodedRegex( upperCase );
+			}
+
+			cur += '(' + regex + ')';
+
+			return cur;
+		}, '' );
+	}
+
+	function createCharacterEncodedRegex( character ) {
+		var map = getCharRegexMap( character ),
+			charRegex = character;
+
+		for ( var code in map ) {
+			if ( map[ code ] ) {
+				charRegex += '|' + map[ code ];
+			}
+		}
+
+		return charRegex;
+	}
+
+	function getCharRegexMap( character ) {
+		var entities = {
+				'<': '&lt;',
+				'>': '&gt;',
+				':': '&colon;'
+			},
+			charCode = character.charCodeAt( 0 ),
+			hex = charCode.toString( 16 );
+
+		return {
+			// `;` is optional and HTML parser is able to recognize codes without it.
+			htmlCode: '&#' + charCode + ';?',
+			// Hexadecimal value is valid despite leading zero padding e.g. `&#x0065` === `&#x65`.
+			hex: '&#x0*' + hex + ';?',
+			entity: entities[ character ]
+		};
+	}
+
 	// Replace all "on\w{3,}" strings which are not:
 	// * opening tags - e.g. `<onfoo`,
 	// * closing tags - e.g. </onfoo> (tested in "false positive 1"),

core/htmlparser/element.js

@@ -163,10 +163,6 @@ CKEDITOR.htmlParser.cssStyle = function() {
 
 			context = element.getFilterContext( context );
 
-			// Do not process elements with data-cke-processor attribute set to off.
-			if ( context.off )
-				return true;
-
 			// Filtering if it's the root node.
 			if ( !element.parent )
 				filter.onRoot( context, element );
@@ -532,15 +528,11 @@ CKEDITOR.htmlParser.cssStyle = function() {
 
 			if ( !ctx ) {
 				ctx = {
-					off: false,
 					nonEditable: false,
 					nestedEditable: false
 				};
 			}
 
-			if ( !ctx.off && this.attributes[ 'data-cke-processor' ] == 'off' )
-				changes.push( 'off', true );
-
 			if ( !ctx.nonEditable && this.attributes.contenteditable == 'false' )
 				changes.push( 'nonEditable', true );
 			// A context to be given nestedEditable must be nonEditable first (by inheritance) (https://dev.ckeditor.com/ticket/11372, https://dev.ckeditor.com/ticket/11698).

tests/core/htmlparser/element.js

@@ -209,19 +209,9 @@ bender.test( {
 		var ctx = el.getFilterContext( undefined );
 
 		assert.isTypeOf( 'object', ctx );
-		assert.isFalse( ctx.off, 'off' );
 		assert.isFalse( ctx.nonEditable, 'nonEditable' );
 	},
 
-	'test getFilterContext - new with property': function() {
-		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p data-cke-processor="off">foo</p>' ).children[ 0 ];
-
-		var ctx = el.getFilterContext( undefined );
-
-		assert.isTypeOf( 'object', ctx );
-		assert.isTrue( ctx.off );
-	},
-
 	'test getFilterContext - no changes': function() {
 		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p>foo</p>' ).children[ 0 ];
 
@@ -231,17 +221,6 @@ bender.test( {
 		assert.areSame( ctx, ctxOld, 'ctx was not modified' );
 	},
 
-	'test getFilterContext - data-cke-processor': function() {
-		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p data-cke-processor="off">foo</p>' ).children[ 0 ];
-
-		var ctxOld = {},
-			ctx = el.getFilterContext( ctxOld );
-
-		assert.isTypeOf( 'object', ctx );
-		assert.isTrue( ctx.off );
-		assert.areNotSame( ctx, ctxOld, 'ctx was cloned' );
-	},
-
 	'test getFilterContext - non-editable': function() {
 		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p contenteditable="false">foo</p>' ).children[ 0 ];
 
@@ -257,26 +236,11 @@ bender.test( {
 		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p>foo</p>' ).children[ 0 ];
 
 		var ctxOld = {
-				off: true,
 				nonEditable: true
 			},
 			ctx = el.getFilterContext( ctxOld );
 
 		assert.areSame( ctx, ctxOld, 'ctx was not modified' );
-		assert.isTrue( ctx.off, 'off' );
-		assert.isTrue( ctx.nonEditable, 'nonEditable' );
-	},
-
-	'test getFilterContext - change - non empty context': function() {
-		var el = new CKEDITOR.htmlParser.fragment.fromHtml( '<p data-cke-processor="off">foo</p>' ).children[ 0 ];
-
-		var ctxOld = {
-				nonEditable: true
-			},
-			ctx = el.getFilterContext( ctxOld );
-
-		assert.areNotSame( ctx, ctxOld, 'ctx was cloned' );
-		assert.isTrue( ctx.off, 'off' );
 		assert.isTrue( ctx.nonEditable, 'nonEditable' );
 	},
 

tests/core/htmlparser/filter.js

@@ -609,46 +609,6 @@ bender.test( {
 		assert.areSame( '<p f="1"><b>foobar</b><i>fuubar</i></p>', writer.getHtml( true ) );
 	},
 
-	'test blocking processing on elements with data-cke-processor="off" attribute': function() {
-		var filter = new CKEDITOR.htmlParser.filter(),
-			writer = new CKEDITOR.htmlParser.basicWriter();
-
-		writer.sortAttributes = true;
-
-		filter.addRules( {
-			root: function( el ) {
-				el.bar = '1';
-			},
-			elements: {
-				span: function( el ) {
-					el.attributes.foo = '1';
-				}
-			},
-			attributeNames: [
-				[ /^a$/, 'b' ]
-			],
-			text: function( value ) {
-				return value + 'X';
-			}
-		} );
-
-		var fragment = CKEDITOR.htmlParser.fragment.fromHtml(
-			'<p><span a="1">1<span a="1">1.1</span><span a="1" data-cke-processor="off">1.2<span>1.2.1</span></span><span a="1">1.3</span></span></p>' );
-		filter.applyTo( fragment );
-		fragment.writeHtml( writer );
-
-		assert.areSame(
-			'<p><span b="1" foo="1">1X<span b="1" foo="1">1.1X</span><span a="1" data-cke-processor="off">1.2<span>1.2.1</span></span><span b="1" foo="1">1.3X</span></span></p>',
-			writer.getHtml( true )
-		);
-
-		var elSpan = CKEDITOR.htmlParser.fragment.fromHtml( '<span a="1" data-cke-processor="off">A</span>' ).children[ 0 ];
-		filter.applyTo( elSpan );
-		elSpan.writeHtml( writer );
-
-		assert.areSame( '<span a="1" data-cke-processor="off">A</span>', writer.getHtml( true ) );
-	},
-
 	'test no processing of non-editable elements': function() {
 		var filter = new CKEDITOR.htmlParser.filter(),
 			writer = new CKEDITOR.htmlParser.basicWriter();