Skip to content

Commit 0cd98dd

Browse files
SimeonCSimeonC
SimeonC
authored and
SimeonC
committed
fix(taBind.formatters): Catch unwrapped content
Fixes #584
1 parent 5d8c44f commit 0cd98dd

7 files changed

+107
-55
lines changed

‎dist/textAngular.min.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/taBind.js

+46-23
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,29 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
8080
var domTest = angular.element("<div>" + value + "</div>");
8181
if(domTest.children().length === 0){
8282
value = "<" + attrs.taDefaultWrap + ">" + value + "</" + attrs.taDefaultWrap + ">";
83+
}else{
84+
var _children = domTest[0].childNodes;
85+
var i;
86+
var _foundBlockElement = false;
87+
for(i = 0; i < _children.length; i++){
88+
if(_foundBlockElement = _children[i].nodeName.toLowerCase().match(BLOCKELEMENTS)) break;
89+
}
90+
if(!_foundBlockElement){
91+
value = "<" + attrs.taDefaultWrap + ">" + value + "</" + attrs.taDefaultWrap + ">";
92+
}else{
93+
value = "";
94+
for(i = 0; i < _children.length; i++){
95+
if(!_children[i].nodeName.toLowerCase().match(BLOCKELEMENTS)){
96+
var _subVal = (_children[i].outerHTML || _children[i].nodeValue);
97+
/* istanbul ignore else: Doesn't seem to trigger on tests, is tested though */
98+
if(_subVal.trim() !== '')
99+
value += "<" + attrs.taDefaultWrap + ">" + _subVal + "</" + attrs.taDefaultWrap + ">";
100+
else value += _subVal;
101+
}else{
102+
value += _children[i].outerHTML;
103+
}
104+
}
105+
}
83106
}
84107
return value;
85108
};
@@ -181,6 +204,29 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
181204
if(!_isReadonly) _setViewValue();
182205
};
183206

207+
// catch DOM XSS via taSanitize
208+
// Sanitizing both ways is identical
209+
var _sanitize = function(unsafe){
210+
return (ngModel.$oldViewValue = taSanitize(taFixChrome(unsafe), ngModel.$oldViewValue, _disableSanitizer));
211+
};
212+
213+
// trigger the validation calls
214+
var _validity = function(value){
215+
if(attrs.required) ngModel.$setValidity('required', !_blankTest(value));
216+
return value;
217+
};
218+
// parsers trigger from the above keyup function or any other time that the viewValue is updated and parses it for storage in the ngModel
219+
ngModel.$parsers.push(_sanitize);
220+
ngModel.$parsers.unshift(_ensureContentWrapped);
221+
ngModel.$parsers.unshift(_validity);
222+
// because textAngular is bi-directional (which is awesome) we need to also sanitize values going in from the server
223+
ngModel.$formatters.push(_sanitize);
224+
ngModel.$formatters.unshift(_ensureContentWrapped);
225+
ngModel.$formatters.unshift(_validity);
226+
ngModel.$formatters.unshift(function(value){
227+
return ngModel.$undoManager.push(value || '');
228+
});
229+
184230
//this code is used to update the models when data is entered/deleted
185231
if(_isInputFriendly){
186232
scope.events = {};
@@ -244,7 +290,6 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
244290
_html += '\n' + _repeat('\t', tablevel-1) + listNode.outerHTML.substring(listNode.outerHTML.lastIndexOf('<'));
245291
return _html;
246292
};
247-
ngModel.$formatters.unshift(_ensureContentWrapped);
248293
ngModel.$formatters.unshift(function(htmlValue){
249294
// tabulate the HTML so it looks nicer
250295
var _children = angular.element('<div>' + htmlValue + '</div>')[0].childNodes;
@@ -581,28 +626,6 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
581626
});
582627
}
583628
}
584-
585-
// catch DOM XSS via taSanitize
586-
// Sanitizing both ways is identical
587-
var _sanitize = function(unsafe){
588-
return (ngModel.$oldViewValue = taSanitize(taFixChrome(unsafe), ngModel.$oldViewValue, _disableSanitizer));
589-
};
590-
591-
// trigger the validation calls
592-
var _validity = function(value){
593-
if(attrs.required) ngModel.$setValidity('required', !_blankTest(value));
594-
return value;
595-
};
596-
// parsers trigger from the above keyup function or any other time that the viewValue is updated and parses it for storage in the ngModel
597-
ngModel.$parsers.push(_sanitize);
598-
ngModel.$parsers.unshift(_validity);
599-
// because textAngular is bi-directional (which is awesome) we need to also sanitize values going in from the server
600-
ngModel.$formatters.push(_sanitize);
601-
ngModel.$formatters.unshift(_ensureContentWrapped);
602-
ngModel.$formatters.unshift(_validity);
603-
ngModel.$formatters.unshift(function(value){
604-
return ngModel.$undoManager.push(value || '');
605-
});
606629

607630
var selectorClickHandler = function(event){
608631
// emit the element-select event, pass the element

‎src/textAngular.js

+46-23
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,29 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
10631063
var domTest = angular.element("<div>" + value + "</div>");
10641064
if(domTest.children().length === 0){
10651065
value = "<" + attrs.taDefaultWrap + ">" + value + "</" + attrs.taDefaultWrap + ">";
1066+
}else{
1067+
var _children = domTest[0].childNodes;
1068+
var i;
1069+
var _foundBlockElement = false;
1070+
for(i = 0; i < _children.length; i++){
1071+
if(_foundBlockElement = _children[i].nodeName.toLowerCase().match(BLOCKELEMENTS)) break;
1072+
}
1073+
if(!_foundBlockElement){
1074+
value = "<" + attrs.taDefaultWrap + ">" + value + "</" + attrs.taDefaultWrap + ">";
1075+
}else{
1076+
value = "";
1077+
for(i = 0; i < _children.length; i++){
1078+
if(!_children[i].nodeName.toLowerCase().match(BLOCKELEMENTS)){
1079+
var _subVal = (_children[i].outerHTML || _children[i].nodeValue);
1080+
/* istanbul ignore else: Doesn't seem to trigger on tests, is tested though */
1081+
if(_subVal.trim() !== '')
1082+
value += "<" + attrs.taDefaultWrap + ">" + _subVal + "</" + attrs.taDefaultWrap + ">";
1083+
else value += _subVal;
1084+
}else{
1085+
value += _children[i].outerHTML;
1086+
}
1087+
}
1088+
}
10661089
}
10671090
return value;
10681091
};
@@ -1164,6 +1187,29 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
11641187
if(!_isReadonly) _setViewValue();
11651188
};
11661189

1190+
// catch DOM XSS via taSanitize
1191+
// Sanitizing both ways is identical
1192+
var _sanitize = function(unsafe){
1193+
return (ngModel.$oldViewValue = taSanitize(taFixChrome(unsafe), ngModel.$oldViewValue, _disableSanitizer));
1194+
};
1195+
1196+
// trigger the validation calls
1197+
var _validity = function(value){
1198+
if(attrs.required) ngModel.$setValidity('required', !_blankTest(value));
1199+
return value;
1200+
};
1201+
// parsers trigger from the above keyup function or any other time that the viewValue is updated and parses it for storage in the ngModel
1202+
ngModel.$parsers.push(_sanitize);
1203+
ngModel.$parsers.unshift(_ensureContentWrapped);
1204+
ngModel.$parsers.unshift(_validity);
1205+
// because textAngular is bi-directional (which is awesome) we need to also sanitize values going in from the server
1206+
ngModel.$formatters.push(_sanitize);
1207+
ngModel.$formatters.unshift(_ensureContentWrapped);
1208+
ngModel.$formatters.unshift(_validity);
1209+
ngModel.$formatters.unshift(function(value){
1210+
return ngModel.$undoManager.push(value || '');
1211+
});
1212+
11671213
//this code is used to update the models when data is entered/deleted
11681214
if(_isInputFriendly){
11691215
scope.events = {};
@@ -1227,7 +1273,6 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
12271273
_html += '\n' + _repeat('\t', tablevel-1) + listNode.outerHTML.substring(listNode.outerHTML.lastIndexOf('<'));
12281274
return _html;
12291275
};
1230-
ngModel.$formatters.unshift(_ensureContentWrapped);
12311276
ngModel.$formatters.unshift(function(htmlValue){
12321277
// tabulate the HTML so it looks nicer
12331278
var _children = angular.element('<div>' + htmlValue + '</div>')[0].childNodes;
@@ -1564,28 +1609,6 @@ angular.module('textAngular.taBind', ['textAngular.factories', 'textAngular.DOM'
15641609
});
15651610
}
15661611
}
1567-
1568-
// catch DOM XSS via taSanitize
1569-
// Sanitizing both ways is identical
1570-
var _sanitize = function(unsafe){
1571-
return (ngModel.$oldViewValue = taSanitize(taFixChrome(unsafe), ngModel.$oldViewValue, _disableSanitizer));
1572-
};
1573-
1574-
// trigger the validation calls
1575-
var _validity = function(value){
1576-
if(attrs.required) ngModel.$setValidity('required', !_blankTest(value));
1577-
return value;
1578-
};
1579-
// parsers trigger from the above keyup function or any other time that the viewValue is updated and parses it for storage in the ngModel
1580-
ngModel.$parsers.push(_sanitize);
1581-
ngModel.$parsers.unshift(_validity);
1582-
// because textAngular is bi-directional (which is awesome) we need to also sanitize values going in from the server
1583-
ngModel.$formatters.push(_sanitize);
1584-
ngModel.$formatters.unshift(_ensureContentWrapped);
1585-
ngModel.$formatters.unshift(_validity);
1586-
ngModel.$formatters.unshift(function(value){
1587-
return ngModel.$undoManager.push(value || '');
1588-
});
15891612

15901613
var selectorClickHandler = function(event){
15911614
// emit the element-select event, pass the element

‎test/taBind/taBind.$formatters.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ describe('taBind.$formatters', function () {
1515
it('adding newlines after immediate child tags', function(){
1616
$rootScope.html = '<p>Test Line 1</p><div>Test Line 2</div><span>Test Line 3</span>';
1717
$rootScope.$digest();
18-
expect(element.val()).toBe('<p>Test Line 1</p>\n<div>Test Line 2</div>\n<span>Test Line 3</span>');
18+
expect(element.val()).toBe('<p>Test Line 1</p>\n<div>Test Line 2</div>\n<p><span>Test Line 3</span></p>');
1919
});
2020
it('ignore nested tags', function(){
2121
$rootScope.html = '<p><b>Test</b> Line 1</p><div>Test <i>Line</i> 2</div><span>Test Line <u>3</u></span>';
2222
$rootScope.$digest();
23-
expect(element.val()).toBe('<p><b>Test</b> Line 1</p>\n<div>Test <i>Line</i> 2</div>\n<span>Test Line <u>3</u></span>');
23+
expect(element.val()).toBe('<p><b>Test</b> Line 1</p>\n<div>Test <i>Line</i> 2</div>\n<p><span>Test Line <u>3</u></span></p>');
2424
});
2525
it('tab out li elements', function(){
2626
$rootScope.html = '<ul><li>Test Line 1</li><li>Test Line 2</li><li>Test Line 3</li></ul>';

‎test/taBind/taBind.sanitize.spec.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ describe('taBind.sanitize', function () {
1919
element.append('<bad-tag>Test 2 Content</bad-tag>');
2020
$rootScope.updateTaBind();
2121
$rootScope.$digest();
22-
expect($rootScope.html).toBe('<p>Test Contents</p>Test 2 Content');
22+
expect($rootScope.html).toBe('<p>Test Contents</p><p>Test 2 Content</p>');
2323
});
2424

2525
it('format from model change', function () {
2626
$rootScope.html += '<bad-tag>Test 2 Content</bad-tag>';
2727
$rootScope.$digest();
28-
expect(element.html()).toBe('<p>Test Contents</p>Test 2 Content');
28+
expect(element.html()).toBe('<p>Test Contents</p><p>Test 2 Content</p>');
2929
});
3030
});
3131

@@ -42,7 +42,7 @@ describe('taBind.sanitize', function () {
4242
element.append('<bad-tag>Test 2 Content</bad-tag>');
4343
$rootScope.updateTaBind();
4444
$rootScope.$digest();
45-
expect($rootScope.html).toBe('<p>Test Contents</p><bad-tag>Test 2 Content</bad-tag>');
45+
expect($rootScope.html).toBe('<p>Test Contents</p><p><bad-tag>Test 2 Content</bad-tag></p>');
4646
});
4747

4848
it('not allow malformed html', function () {

‎test/taBind/taBind.spec.js

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/textAngular.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ describe('textAngular', function(){
611611
element.append('<bad-tag>Test 2 Content</bad-tag>');
612612
element.triggerHandler('keyup');
613613
$rootScope.$digest();
614-
expect(element2.val()).toBe('<p>Test Contents</p>\n<bad-tag>Test 2 Content</bad-tag>');
614+
expect(element2.val()).toBe('<p>Test Contents</p>\n<p><bad-tag>Test 2 Content</bad-tag></p>');
615615
});
616616

617617
it('not allow malformed html', function () {
@@ -636,7 +636,7 @@ describe('textAngular', function(){
636636
element.append('<bad-tag>Test 2 Content</bad-tag>');
637637
element.triggerHandler('keyup');
638638
$rootScope.$digest();
639-
expect($rootScope.html).toBe('<p>Test Contents</p><bad-tag>Test 2 Content</bad-tag>');
639+
expect($rootScope.html).toBe('<p>Test Contents</p><p><bad-tag>Test 2 Content</bad-tag></p>');
640640
});
641641

642642
it('not allow malformed html', function () {

0 commit comments

Comments
 (0)