Fix sanitization issues as suggested by evn

author: Misko Hevery 2010-12-03 15:42:11 -0800
committer: Misko Hevery 2010-12-03 15:42:42 -0800
commit: 2bbced212e2ee93948c45360fee00b2e3f960392 (patch)
tree: a42ea9b49c42c37b9f8e42fa0fc4bf1fb906f948
parent: 5a8ad8fe329fc09898ff43a060710265d38393be (diff)
download: angular.js-2bbced212e2ee93948c45360fee00b2e3f960392.tar.bz2
4 files changed, 86 insertions, 25 deletions
diff --git a/regression/issue-169.html b/regression/issue-169.html
new file mode 100644
index 00000000..e18c4f2e
--- /dev/null
+++ b/regression/issue-169.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html xmlns:ng="http://angularjs.org">
+  <script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
+  <body>
+    <span ng:init='x = {d:3}; x1 = {bar:[x,5]}; x1.bar[0].d = 4'>
+        <input name="x1.bar[0].d" type="text"></input>
+        <input name="x.d" type="text"></input>
+    <span> {{x1}} -- {{x1.bar[0].d}}</span>
+  </body>
+</html>
+\ No newline at end of file
diff --git a/regression/sanitizer.html b/regression/sanitizer.html
new file mode 100644
index 00000000..775a6009
--- /dev/null
+++ b/regression/sanitizer.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html xmlns:ng="http://angularjs.org">
+  <script type="text/javascript" src="../src/angular-bootstrap.js" ng:autobind></script>
+  <body>
+   <textarea name="html" rows="10" cols="100"></textarea>
+   <div>{{html|html}}</div>
+  </body>
+</html>
+\ No newline at end of file
diff --git a/src/sanitizer.js b/src/sanitizer.js
index 66631a90..d63cf69d 100644
--- a/src/sanitizer.js
+++ b/src/sanitizer.js
@@ -17,7 +17,7 @@
 // Regular Expressions for parsing tags and attributes
 var START_TAG_REGEXP = /^<\s*([\w:]+)((?:\s+\w+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|[^>\s]+))?)*)\s*(\/?)\s*>/,
   END_TAG_REGEXP = /^<\s*\/\s*([\w:]+)[^>]*>/,
-  ATTR_REGEXP = /(\w+)(?:\s*=\s*(?:(?:"((?:\\.|[^"])*)")|(?:'((?:\\.|[^'])*)')|([^>\s]+)))?/g,
+  ATTR_REGEXP = /(\w+)(?:\s*=\s*(?:(?:"((?:[^"])*)")|(?:'((?:[^'])*)')|([^>\s]+)))?/g,
   BEGIN_TAG_REGEXP = /^</,
   BEGING_END_TAGE_REGEXP = /^<\s*\//,
   COMMENT_REGEXP = /<!--(.*?)-->/g,
@@ -26,32 +26,32 @@ var START_TAG_REGEXP = /^<\s*([\w:]+)((?:\s+\w+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']
   NON_ALPHANUMERIC_REGEXP = /([^\#-~| |!])/g; // Match everything outside of normal chars and " (quote character)
 
 // Empty Elements - HTML 4.01
-var emptyElements = makeMap("area,base,basefont,br,col,hr,img,input,isindex,link,param");
+var emptyElements = makeMap("area,br,col,hr,img");
 
 // Block Elements - HTML 4.01
-var blockElements = makeMap("address,blockquote,button,center,dd,del,dir,div,dl,dt,fieldset,"+
-    "form,hr,ins,isindex,li,map,menu,ol,p,pre,script,table,tbody,td,tfoot,th,thead,tr,ul");
+var blockElements = makeMap("address,blockquote,center,dd,del,dir,div,dl,dt,"+
+    "hr,ins,li,map,menu,ol,p,pre,script,table,tbody,td,tfoot,th,thead,tr,ul");
 
 // Inline Elements - HTML 4.01
-var inlineElements = makeMap("a,abbr,acronym,b,basefont,bdo,big,br,button,cite,code,del,dfn,em,font,i,img,"+
-    "input,ins,kbd,label,map,q,s,samp,select,small,span,strike,strong,sub,sup,textarea,tt,u,var");
+var inlineElements = makeMap("a,abbr,acronym,b,bdo,big,br,cite,code,del,dfn,em,font,i,img,"+
+    "ins,kbd,label,map,q,s,samp,small,span,strike,strong,sub,sup,tt,u,var");
 // Elements that you can, intentionally, leave open
 // (and which close themselves)
-var closeSelfElements = makeMap("colgroup,dd,dt,li,options,p,td,tfoot,th,thead,tr");
+var closeSelfElements = makeMap("colgroup,dd,dt,li,p,td,tfoot,th,thead,tr");
 // Special Elements (can contain anything)
 var specialElements = makeMap("script,style");
 var validElements = extend({}, emptyElements, blockElements, inlineElements, closeSelfElements);
 
 //see: http://www.w3.org/TR/html4/index/attributes.html
 //Attributes that have their values filled in disabled="disabled"
-var fillAttrs = makeMap("checked,compact,declare,defer,disabled,ismap,multiple,nohref,noresize,noshade,nowrap,readonly,selected");
+var fillAttrs = makeMap("compact,ismap,nohref,nowrap");
 //Attributes that have href and hence need to be sanitized
 var uriAttrs = makeMap("background,href,longdesc,src,usemap");
 var validAttrs = extend({}, fillAttrs, uriAttrs, makeMap(
     'abbr,align,alt,axis,bgcolor,border,cellpadding,cellspacing,class,clear,'+
-    'color,cols,colspan,coords,dir,face,for,headers,height,hreflang,hspace,id,'+
-    'label,lang,language,maxlength,method,name,prompt,rel,rev,rows,rowspan,rules,'+
-    'scope,scrolling,shape,size,span,start,summary,tabindex,target,title,type,'+
+    'color,cols,colspan,coords,dir,face,headers,height,hreflang,hspace,'+
+    'lang,language,rel,rev,rows,rowspan,rules,'+
+    'scope,scrolling,shape,span,start,summary,target,title,type,'+
     'valign,value,vspace,width'));
 
 /**
@@ -249,12 +249,12 @@ function htmlSanitizeWriter(buf){
       if (!ignore && specialElements[tag]) {
         ignore = tag;
       }
-      if (!ignore && validElements[tag]) {
+      if (!ignore && validElements[tag] == true) {
         out('<');
         out(tag);
         foreach(attrs, function(value, key){
           var lkey=lowercase(key);
-          if (validAttrs[lkey] && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
+          if (validAttrs[lkey]==true && (uriAttrs[lkey]!==true || value.match(URI_REGEXP))) {
             out(' ');
             out(key);
             out('="');
@@ -267,7 +267,7 @@ function htmlSanitizeWriter(buf){
     },
     end: function(tag){
         tag = lowercase(tag);
-        if (!ignore && validElements[tag]) {
+        if (!ignore && validElements[tag] == true) {
           out('</');
           out(tag);
           out('>');
diff --git a/test/sanitizerSpec.js b/test/sanitizerSpec.js
index 88da693d..3ad6c1c9 100644
--- a/test/sanitizerSpec.js
+++ b/test/sanitizerSpec.js
@@ -33,7 +33,7 @@ describe('HTML', function(){
     expectHTML('a<SCRIPT>ev<script>evil</sCript>il</scrIpt>c.').toEqual('ac.');
   });
 
-  it('should remove unknown tag names', function(){
+  it('should remove unknown  names', function(){
     expectHTML('a<xxx><B>b</B></xxx>c').toEqual('a<b>b</b>c');
   });
 
@@ -50,21 +50,33 @@ describe('HTML', function(){
   });
 
   it('should handle entities', function(){
-    var everything = '<div id="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;">' + 
+    var everything = '<div rel="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;">' + 
     '!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#295;</div>';
     expectHTML(everything).toEqual(everything);
   });
   
   it('should handle improper html', function(){
-    expectHTML('< div id="</div>" alt=abc dir=\'"\' >text< /div>').
-      toEqual('<div id="&lt;/div&gt;" alt="abc" dir="&#34;">text</div>');
+    expectHTML('< div rel="</div>" alt=abc dir=\'"\' >text< /div>').
+      toEqual('<div rel="&lt;/div&gt;" alt="abc" dir="&#34;">text</div>');
   });
 
   it('should handle improper html2', function(){
-    expectHTML('< div id="</div>" / >').
-      toEqual('<div id="&lt;/div&gt;"/>');
+    expectHTML('< div rel="</div>" / >').
+      toEqual('<div rel="&lt;/div&gt;"/>');
   });
-
+  
+  it('should ignore back slash as escape', function(){
+    expectHTML('<img alt="xxx\\" title="><script>....">').
+      toEqual('<img alt="xxx\\" title="&gt;&lt;script&gt;...."/>');
+  });
+  
+  it('should ignore object attributes', function(){
+    expectHTML('<a constructor="hola">:)</a>').
+      toEqual('<a>:)</a>');
+    expectHTML('<constructor constructor="hola">:)</constructor>').
+      toEqual('');
+  });
+  
   describe('htmlSanitizerWriter', function(){
     var writer, html;
     beforeEach(function(){
@@ -74,12 +86,12 @@ describe('HTML', function(){
 
     it('should write basic HTML', function(){
       writer.chars('before');
-      writer.start('div', {id:'123'}, false);
+      writer.start('div', {rel:'123'}, false);
       writer.chars('in');
       writer.end('div');
       writer.chars('after');
 
-      expect(html).toEqual('before<div id="123">in</div>after');
+      expect(html).toEqual('before<div rel="123">in</div>after');
     });
 
     it('should escape text nodes', function(){
@@ -93,8 +105,8 @@ describe('HTML', function(){
     });
 
     it('should escape attributes', function(){
-      writer.start('div', {id:'!@#$%^&*()_+-={}[]:";\'<>?,./`~ \n\0\r\u0127'});
-      expect(html).toEqual('<div id="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#10;&#0;&#13;&#295;">');
+      writer.start('div', {rel:'!@#$%^&*()_+-={}[]:";\'<>?,./`~ \n\0\r\u0127'});
+      expect(html).toEqual('<div rel="!@#$%^&amp;*()_+-={}[]:&#34;;\'&lt;&gt;?,./`~ &#10;&#0;&#13;&#295;">');
     });
 
     it('should ignore missformed elements', function(){
@@ -107,6 +119,37 @@ describe('HTML', function(){
       expect(html).toEqual('<div>');
     });
     
+    describe('explicitly dissallow', function(){
+      it('should not allow attributes', function(){
+        writer.start('div', {id:'a', name:'a', style:'a'});
+        expect(html).toEqual('<div>');
+      });
+      
+      it('should not allow tags', function(){
+        function tag(name) {
+          writer.start(name, {});
+          writer.end(name);
+        };
+        tag('frameset');
+        tag('frame');
+        tag('form');
+        tag('param');
+        tag('object');
+        tag('embed');
+        tag('textarea');
+        tag('input');
+        tag('button');
+        tag('option');
+        tag('select');
+        tag('script');
+        tag('style');
+        tag('link');
+        tag('base');
+        tag('basefont');
+        expect(html).toEqual('');
+      });
+    });
+    
     describe('isUri', function(){
       
       function isUri(value) {
author	Misko Hevery	2010-12-03 15:42:11 -0800
committer	Misko Hevery	2010-12-03 15:42:42 -0800
commit	2bbced212e2ee93948c45360fee00b2e3f960392 (patch)
tree	a42ea9b49c42c37b9f8e42fa0fc4bf1fb906f948
parent	5a8ad8fe329fc09898ff43a060710265d38393be (diff)
download	angular.js-2bbced212e2ee93948c45360fee00b2e3f960392.tar.bz2