Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 2bbced2

Browse files
mheverymhevery
committed
Fix sanitization issues as suggested by evn
1 parent 5a8ad8f commit 2bbced2

File tree

4 files changed

+86
-25
lines changed

4 files changed

+86
-25
lines changed

Diff for: ‎regression/issue-169.html

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!DOCTYPE HTML>
2+
<html xmlns:ng="http://angularjs.org">
3+
<script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
4+
<body>
5+
<span ng:init='x = {d:3}; x1 = {bar:[x,5]}; x1.bar[0].d = 4'>
6+
<input name="x1.bar[0].d" type="text"></input>
7+
<input name="x.d" type="text"></input>
8+
<span> {{x1}} -- {{x1.bar[0].d}}</span>
9+
</body>
10+
</html>

Diff for: ‎regression/sanitizer.html

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<!DOCTYPE HTML>
2+
<html xmlns:ng="http://angularjs.org">
3+
<script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
4+
<body>
5+
<textarea name="html" rows="10" cols="100"></textarea>
6+
<div>{{html|html}}</div>
7+
</body>
8+
</html>

Diff for: ‎src/sanitizer.js

+14-14
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// Regular Expressions for parsing tags and attributes
1818
var START_TAG_REGEXP = /^<\s*([\w:]+)((?:\s+\w+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|[^>\s]+))?)*)\s*(\/?)\s*>/,
1919
END_TAG_REGEXP = /^<\s*\/\s*([\w:]+)[^>]*>/,
20-
ATTR_REGEXP = /(\w+)(?:\s*=\s*(?:(?:"((?:\\.|[^"])*)")|(?:'((?:\\.|[^'])*)')|([^>\s]+)))?/g,
20+
ATTR_REGEXP = /(\w+)(?:\s*=\s*(?:(?:"((?:[^"])*)")|(?:'((?:[^'])*)')|([^>\s]+)))?/g,
2121
BEGIN_TAG_REGEXP = /^</,
2222
BEGING_END_TAGE_REGEXP = /^<\s*\//,
2323
COMMENT_REGEXP = /<!--(.*?)-->/g,
@@ -26,32 +26,32 @@ var START_TAG_REGEXP = /^<\s*([\w:]+)((?:\s+\w+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']
2626
NON_ALPHANUMERIC_REGEXP = /([^\#-~| |!])/g; // Match everything outside of normal chars and " (quote character)
2727

2828
// Empty Elements - HTML 4.01
29-
var emptyElements = makeMap("area,base,basefont,br,col,hr,img,input,isindex,link,param");
29+
var emptyElements = makeMap("area,br,col,hr,img");
3030

3131
// Block Elements - HTML 4.01
32-
var blockElements = makeMap("address,blockquote,button,center,dd,del,dir,div,dl,dt,fieldset,"+
33-
"form,hr,ins,isindex,li,map,menu,ol,p,pre,script,table,tbody,td,tfoot,th,thead,tr,ul");
32+
var blockElements = makeMap("address,blockquote,center,dd,del,dir,div,dl,dt,"+
33+
"hr,ins,li,map,menu,ol,p,pre,script,table,tbody,td,tfoot,th,thead,tr,ul");
3434

3535
// Inline Elements - HTML 4.01
36-
var inlineElements = makeMap("a,abbr,acronym,b,basefont,bdo,big,br,button,cite,code,del,dfn,em,font,i,img,"+
37-
"input,ins,kbd,label,map,q,s,samp,select,small,span,strike,strong,sub,sup,textarea,tt,u,var");
36+
var inlineElements = makeMap("a,abbr,acronym,b,bdo,big,br,cite,code,del,dfn,em,font,i,img,"+
37+
"ins,kbd,label,map,q,s,samp,small,span,strike,strong,sub,sup,tt,u,var");
3838
// Elements that you can, intentionally, leave open
3939
// (and which close themselves)
40-
var closeSelfElements = makeMap("colgroup,dd,dt,li,options,p,td,tfoot,th,thead,tr");
40+
var closeSelfElements = makeMap("colgroup,dd,dt,li,p,td,tfoot,th,thead,tr");
4141
// Special Elements (can contain anything)
4242
var specialElements = makeMap("script,style");
4343
var validElements = extend({}, emptyElements, blockElements, inlineElements, closeSelfElements);
4444

4545
//see: http://www.w3.org/TR/html4/index/attributes.html
4646
//Attributes that have their values filled in disabled="disabled"
47-
var fillAttrs = makeMap("checked,compact,declare,defer,disabled,ismap,multiple,nohref,noresize,noshade,nowrap,readonly,selected");
47+
var fillAttrs = makeMap("compact,ismap,nohref,nowrap");
4848
//Attributes that have href and hence need to be sanitized
4949
var uriAttrs = makeMap("background,href,longdesc,src,usemap");
5050
var validAttrs = extend({}, fillAttrs, uriAttrs, makeMap(
5151
'abbr,align,alt,axis,bgcolor,border,cellpadding,cellspacing,class,clear,'+
52-
'color,cols,colspan,coords,dir,face,for,headers,height,hreflang,hspace,id,'+
53-
'label,lang,language,maxlength,method,name,prompt,rel,rev,rows,rowspan,rules,'+
54-
'scope,scrolling,shape,size,span,start,summary,tabindex,target,title,type,'+
52+
'color,cols,colspan,coords,dir,face,headers,height,hreflang,hspace,'+
53+
'lang,language,rel,rev,rows,rowspan,rules,'+
54+
'scope,scrolling,shape,span,start,summary,target,title,type,'+
5555
'valign,value,vspace,width'));
5656

5757
/**
@@ -249,12 +249,12 @@ function htmlSanitizeWriter(buf){
249249
if (!ignore && specialElements[tag]) {
250250
ignore = tag;
251251
}
252-
if (!ignore && validElements[tag]) {
252+
if (!ignore && validElements[tag] == true) {
253253
out('<');
254254
out(tag);
255255
foreach(attrs, function(value, key){
256256
var lkey=lowercase(key);
257-
if (validAttrs[lkey] && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
257+
if (validAttrs[lkey]==true && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
258258
out(' ');
259259
out(key);
260260
out('="');
@@ -267,7 +267,7 @@ function htmlSanitizeWriter(buf){
267267
},
268268
end: function(tag){
269269
tag = lowercase(tag);
270-
if (!ignore && validElements[tag]) {
270+
if (!ignore && validElements[tag] == true) {
271271
out('</');
272272
out(tag);
273273
out('>');

Diff for: ‎test/sanitizerSpec.js

+54-11
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('HTML', function(){
3333
expectHTML('a<SCRIPT>ev<script>evil</sCript>il</scrIpt>c.').toEqual('ac.');
3434
});
3535

36-
it('should remove unknown tag names', function(){
36+
it('should remove unknown names', function(){
3737
expectHTML('a<xxx><B>b</B></xxx>c').toEqual('a<b>b</b>c');
3838
});
3939

@@ -50,21 +50,33 @@ describe('HTML', function(){
5050
});
5151

5252
it('should handle entities', function(){
53-
var everything = '<div id="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;">' +
53+
var everything = '<div rel="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;">' +
5454
'!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;</div>';
5555
expectHTML(everything).toEqual(everything);
5656
});
5757

5858
it('should handle improper html', function(){
59-
expectHTML('< div id="</div>" alt=abc dir=\'"\' >text< /div>').
60-
toEqual('<div id="&lt;/div&gt;" alt="abc" dir="&#34;">text</div>');
59+
expectHTML('< div rel="</div>" alt=abc dir=\'"\' >text< /div>').
60+
toEqual('<div rel="&lt;/div&gt;" alt="abc" dir="&#34;">text</div>');
6161
});
6262

6363
it('should handle improper html2', function(){
64-
expectHTML('< div id="</div>" / >').
65-
toEqual('<div id="&lt;/div&gt;"/>');
64+
expectHTML('< div rel="</div>" / >').
65+
toEqual('<div rel="&lt;/div&gt;"/>');
6666
});
67-
67+
68+
it('should ignore back slash as escape', function(){
69+
expectHTML('<img alt="xxx\\" title="><script>....">').
70+
toEqual('<img alt="xxx\\" title="&gt;&lt;script&gt;...."/>');
71+
});
72+
73+
it('should ignore object attributes', function(){
74+
expectHTML('<a constructor="hola">:)</a>').
75+
toEqual('<a>:)</a>');
76+
expectHTML('<constructor constructor="hola">:)</constructor>').
77+
toEqual('');
78+
});
79+
6880
describe('htmlSanitizerWriter', function(){
6981
var writer, html;
7082
beforeEach(function(){
@@ -74,12 +86,12 @@ describe('HTML', function(){
7486

7587
it('should write basic HTML', function(){
7688
writer.chars('before');
77-
writer.start('div', {id:'123'}, false);
89+
writer.start('div', {rel:'123'}, false);
7890
writer.chars('in');
7991
writer.end('div');
8092
writer.chars('after');
8193

82-
expect(html).toEqual('before<div id="123">in</div>after');
94+
expect(html).toEqual('before<div rel="123">in</div>after');
8395
});
8496

8597
it('should escape text nodes', function(){
@@ -93,8 +105,8 @@ describe('HTML', function(){
93105
});
94106

95107
it('should escape attributes', function(){
96-
writer.start('div', {id:'!@#$%^&*()_+-={}[]:";\'<>?,./`~ \n\0\r\u0127'});
97-
expect(html).toEqual('<div id="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#10;&#0;&#13;&#295;">');
108+
writer.start('div', {rel:'!@#$%^&*()_+-={}[]:";\'<>?,./`~ \n\0\r\u0127'});
109+
expect(html).toEqual('<div rel="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#10;&#0;&#13;&#295;">');
98110
});
99111

100112
it('should ignore missformed elements', function(){
@@ -107,6 +119,37 @@ describe('HTML', function(){
107119
expect(html).toEqual('<div>');
108120
});
109121

122+
describe('explicitly dissallow', function(){
123+
it('should not allow attributes', function(){
124+
writer.start('div', {id:'a', name:'a', style:'a'});
125+
expect(html).toEqual('<div>');
126+
});
127+
128+
it('should not allow tags', function(){
129+
function tag(name) {
130+
writer.start(name, {});
131+
writer.end(name);
132+
};
133+
tag('frameset');
134+
tag('frame');
135+
tag('form');
136+
tag('param');
137+
tag('object');
138+
tag('embed');
139+
tag('textarea');
140+
tag('input');
141+
tag('button');
142+
tag('option');
143+
tag('select');
144+
tag('script');
145+
tag('style');
146+
tag('link');
147+
tag('base');
148+
tag('basefont');
149+
expect(html).toEqual('');
150+
});
151+
});
152+
110153
describe('isUri', function(){
111154

112155
function isUri(value) {

0 commit comments

Comments
 (0)