patch(fix #85): better XSS fix (#87)

* patch(fix #85): fixes xss vulnerability * patch(fix #85): better XSS fix
wickedest · Jun 23, 2018 · 217674c · 217674c
1 parent 3812dd6
commit 217674c
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 34 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "mergely",
-  "version": "4.0.4",
+  "version": "4.0.5",
   "description": "A javascript UI for diff/merge",
   "directories": {
     "doc": "doc",

diff --git a/src/mergely.js b/src/mergely.js
@@ -659,6 +659,13 @@ jQuery.extend(Mgly.CodeMirrorDiffView.prototype, {
 	bind: function(el) {
 		this.element.hide();
 		this.id = jQuery(el).attr('id');
+		try {
+			// ensure the id is valid for jQuery
+			jQuery(`#${this.id}`);
+		} catch (ex) {
+			console.error(`jQuery failed to find mergely: #${this.id}`);
+			return;
+		}
 		this.changed_timeout = null;
 		this.chfns = {};
 		this.chfns[this.id + '-lhs'] = [];
@@ -749,32 +756,42 @@ jQuery.extend(Mgly.CodeMirrorDiffView.prototype, {
 			});
 		}
 
+		// check initialization
+		var rhstx;
+		try {
+			rhstx = this.element.find(`#${this.id}-rhs`).get(0);
+		} catch (ex) {
+		}
+		if (!rhstx) {
+			console.error('rhs textarea not defined - Mergely not initialized properly');
+			return;
+		}
+		var lhstx;
+		try {
+			lhstx = this.element.find(`#${this.id}-lhs`).get(0);
+		} catch (ex) {
+		}
+		if (!lhstx) {
+			console.error('lhs textarea not defined - Mergely not initialized properly');
+			return;
+		}
+
 		// get current diff border color
 		var color = jQuery('<div style="display:none" class="mergely current start" />').appendTo('body').css('border-top-color');
 		this.current_diff_color = color;
 
 		// codemirror
-		var cmstyle = '#' + this.id + ' .CodeMirror-gutter-text { padding: 5px 0 0 0; }' +
-			'#' + this.id + ' .CodeMirror-lines pre, ' + '#' + this.id + ' .CodeMirror-gutter-text pre { line-height: 18px; }' +
-			'.CodeMirror-linewidget { overflow: hidden; };';
+		var cmstyle = `#${this.id} .CodeMirror-gutter-text { padding: 5px 0 0 0; }
+			'#${this.id} .CodeMirror-lines pre, #${this.id} .CodeMirror-gutter-text pre { line-height: 18px; }
+			'.CodeMirror-linewidget { overflow: hidden; };`;
 		if (this.settings.autoresize) {
-			cmstyle += this.id + ' .CodeMirror-scroll { height: 100%; overflow: auto; }';
+			cmstyle += `${this.id} .CodeMirror-scroll { height: 100%; overflow: auto; }`;
 		}
 		// adjust the margin line height
 		cmstyle += '\n.CodeMirror { line-height: 18px; }';
-		jQuery('<style type="text/css">' + cmstyle + '</style>').appendTo('head');
+		jQuery(`<style type="text/css">${cmstyle}</style>`).appendTo('head');
 
-		//bind
-		var rhstx = this.element.find('#' + this.id + '-rhs').get(0);
-		if (!rhstx) {
-			console.error('rhs textarea not defined - Mergely not initialized properly');
-			return;
-		}
-		var lhstx = this.element.find('#' + this.id + '-lhs').get(0);
-		if (!rhstx) {
-			console.error('lhs textarea not defined - Mergely not initialized properly');
-			return;
-		}
+		// bind
 		var self = this;
 		this.editor = [];
 		this.editor[this.id + '-lhs'] = CodeMirror.fromTextArea(lhstx, this.lhs_cmsettings);

diff --git a/tests/mergely.spec.js b/tests/mergely.spec.js
@@ -20,9 +20,10 @@ describe('mergely', function () {
 	};
 
 	afterEach(() => {
-		$('#mergely').mergely('unbind');
-		$('#mergely').mergelyUnregister();
-		$('#mergely').remove();
+		const mergely = $('#mergely');
+		mergely.mergely('unbind');
+		mergely.mergelyUnregister();
+		mergely.remove();
 	});
 
 	describe('initialization', () => {
@@ -525,13 +526,11 @@ describe('mergely', function () {
 			});
 		});
 
-		it.only('should not be vulnerable to XSS', function (done) {
+		it('should not be vulnerable to XSS', function (done) {
 			function initXSS(options) {
-				// $('body').css({'margin': '0px'}).append("<div id='mergely<script>alert(123)</script>' />");
-
-				$('body').get(0).innerHTML = "<div id='mergely\"<script id='injected'>alert(123)</script>'></div>";
-
-				const editor = $('#mergely');
+				$('body').get(0).innerHTML = "<!DOCTYPE html><html lang=\"en\"><body><div id='mergely<script id=\"injected\">alert(123)</script>'></div></body></html>";
+				const divs = document.getElementsByTagName('div');
+				editor = $(divs[0]);
 				editor.mergely(options);
 				return editor;
 			};
@@ -544,14 +543,10 @@ describe('mergely', function () {
 					lhs: (setValue) => setValue(macbeth),
 					rhs: (setValue) => setValue(macbeth)
 				});
-				const { mergely } = $('#mergely');
-				// console.log('HERE', $('body').html());
-				// const { mergely } = $('#mergely&quot;<script>alert(123)</script>');
-				// expect($('#mergely<script>alert(123)</script>').mergely('_is_change_in_view', 'lhs', {from: 10, to: 20}, {
-				// 	'lhs-line-from': 0,
-				// 	'lhs-line-to': 25
-				// })).to.be.true;
-				expect($('body').find('#injected')).to.have.length(0);
+				expect($('body').find('#injected')).to.have.length(0, 'expected no div with id injected');
+				const divs = document.getElementsByTagName('div');
+				expect(divs).to.have.length(1);
+				expect(divs[0].id).to.equal('mergely<script id="injected">alert(123)</script>');
 				done();
 			});
 		});