Merge lp:~vauxoo/openobject-server/7.0-backport_new_html_sanitize into lp:openobject-server/7.0

[Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com]

Overwritten method html_sanitizer from tools, to add 3 new features:

1) Avoid render a link to hosts not allowed, for this we add a new configuration with a host allow list in ir.configuration model

2) Avoide render a link with a scheme not allowed, also added a list with schemes allow like https in ir.configuration model

3) Backport new features from trunk branch

[Olivier Dony (Odoo) (odo-openerp)]

Thanks for the proposal!
Assuming this MP is targeted to 7.0, you need to re-submit it with the correct target branch, which will solve the huge diff/conflicts problem.
Now, based on the summary and diff it seems more like an improvement/new feature than a bugfix, so it should actually be for trunk and not for 7.0.

Note: calling the normal eval() on a user-provided string is very dangerous, you should use ast.literal_eval or safe_eval.expr_eval.

Thanks,

review: Needs Resubmitting ((surface scan))

[Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com (josemoralesp)]

You will need this library, that I overwrite

[lp:~vauxoo/openobject-server/7.0-backport_new_html_sanitize]

5082. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-19

[IMP] Added inpect to know the elements for method allowed

5083. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-19

[IMP]

Unmerged revisions

5083. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-19

[IMP]

5082. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-19

[IMP] Added inpect to know the elements for method allowed

5081. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-18

[IMP] Change eval for safe_eval to better the code

5080. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-18

[IMP] Avoid apply eval if not exist configuration

5079. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-18

[IMP] Overwritten method html_sanitizer from tools, to add 3 new features:

1) Avoid render a link to hosts not allowed, for this we add a new configuration with a host allow list in ir.configuration model

2) Avoide render a link with a scheme not allowed, also added a list with schemes allow like https in ir.configuration model

3) Backport new features from trunk branch

5078. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-09-17

[IMP] Backport from trunk to add new features in html_sanitize method

=== modified file 'openerp/tools/mail.py'
--- openerp/tools/mail.py	2013-03-29 15:41:30 +0000
+++ openerp/tools/mail.py	2013-09-19 15:03:35 +0000
@@ -20,6 +20,7 @@
 ##############################################################################
 
 from lxml import etree
+from openerp.tools.safe_eval import safe_eval
 import cgi
 import logging
 import lxml.html
@@ -29,7 +30,16 @@
 import re
 import socket
 import threading
+from lxml.html import xhtml_to_html, _transform_result
+
+try:                                                                                          
+    from urlparse import urlsplit                                                             
+except ImportError:                                                                           
+    # Python3                                                                                 
+    from urllib.parse import urlsplit    
 import time
+import inspect
+from openerp import SUPERUSER_ID
 
 from openerp.loglevels import ustr
 
@@ -43,28 +53,94 @@
 tags_to_kill = ["script", "head", "meta", "title", "link", "style", "frame", "iframe", "base", "object", "embed"]
 tags_to_remove = ['html', 'body', 'font']
 
-
-def html_sanitize(src):
+# allow new semantic HTML5 tags
+allowed_tags = clean.defs.tags | frozenset('article section header footer hgroup nav aside figure'.split())
+safe_attrs = clean.defs.safe_attrs | frozenset(['style'])
+
+
+
+def html_sanitize(src, silent=True):
     if not src:
         return src
+    logger = logging.getLogger(__name__ + '.html_sanitize')
+    cr = False
+    host_whitelist = False
+    scheme_allowed = False
+    try:
+        db_name = getattr(threading.currentThread(), 'dbname', None)
+        if db_name:
+            local_cr = cr = pooler.get_db(db_name).cursor()
+    except:
+        logger.warning.error("Data Base not found")
+
     src = ustr(src, errors='replace')
 
+
     # html encode email tags
     part = re.compile(r"(<(([^a<>]|a[^<>\s])[^<>]*)@[^<>]+>)", re.IGNORECASE | re.DOTALL)
     src = part.sub(lambda m: cgi.escape(m.group(1)), src)
-    
-    # some corner cases make the parser crash (such as <SCRIPT/XSS SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT> in test_mail)
+
+    if cr:
+        ir_config_pool = pooler.get_pool(cr.dbname).get('ir.config_parameter')
+        host_whitelist = ir_config_pool.get_param(cr, SUPERUSER_ID, 'host_whitelist')
+        host_whitelist = host_whitelist and safe_eval(host_whitelist)
+        scheme_allowed = ir_config_pool.get_param(cr, SUPERUSER_ID, 'scheme_allowed')
+        scheme_allowed = scheme_allowed and safe_eval(scheme_allowed)
+
+    kwargs = {
+        'page_structure': True,
+        'whitelist_tags': None, # do not exclude url for their tag
+        'host_whitelist': host_whitelist or (),
+        'style': False, # do not remove style attributes
+        'forms': True,  # remove form tags
+        'remove_unknown_tags': False,
+        'allow_tags': allowed_tags,
+    }
+    if etree.LXML_VERSION >= (2, 3, 1):
+        # kill_tags attribute has been added in version 2.3.1
+        kwargs.update({
+            'kill_tags': tags_to_kill,
+            'remove_tags': tags_to_remove,
+        })
+    else:
+        kwargs['remove_tags'] = tags_to_kill + tags_to_remove
+
+    if etree.LXML_VERSION >= (3, 1, 0):
+        kwargs.update({
+            'safe_attrs_only': True,
+            'safe_attrs': safe_attrs,
+        })
+    else:
+        # lxml < 3.1.0 does not allow to specify safe_attrs. We keep all attributes in order to keep "style"
+        kwargs['safe_attrs_only'] = False
+
     try:
-        cleaner = clean.Cleaner(page_structure=True, style=False, safe_attrs_only=False, forms=False, kill_tags=tags_to_kill, remove_tags=tags_to_remove)
-        cleaned = cleaner.clean_html(src)
-    except TypeError, e:
-        # lxml.clean version < 2.3.1 does not have a kill_tags attribute
-        # to remove in 2014
-        cleaner = clean.Cleaner(page_structure=True, style=False, safe_attrs_only=False, forms=False, remove_tags=tags_to_kill+tags_to_remove)
-        cleaned = cleaner.clean_html(src)
-    except:
-        _logger.warning('html_sanitize failed to parse %s' % (src))
-        cleaned = '<p>Impossible to parse</p>'
+        # some corner cases make the parser crash (such as <SCRIPT/XSS SRC=\"http://ha.ckers.org/xss.js\"></SCRIPT> in test_mail)
+        cleaner = clean.Cleaner(**kwargs)
+        body = lxml.html.document_fromstring(src)
+        for el, attrib, link, pos in body.iterlinks():
+            scheme, netloc, path, query, fragment = urlsplit(link)
+            elements = inspect.getargspec(cleaner.allow_embedded_url)
+            elements = elements.args
+            if len(elements) > 3 and not \
+                    cleaner.allow_embedded_url(el, link, scheme_allowed) or \
+                    cleaner.allow_embedded_url(el, link) and \
+                    scheme and el.tag != 'a':
+                el.getparent().replace(el, clean.fromstring(cgi.escape(clean.tostring(el)))) 
+        cleaned = cleaner.clean_html(body)
+        if not isinstance(cleaned, basestring):
+            cleaned = _transform_result(unicode, cleaned) 
+
+    except etree.ParserError:
+        if not silent:
+            raise
+        logger.warning('ParserError obtained when sanitizing %r', src, exc_info=True)
+        cleaned = '<p>ParserError when sanitizing</p>'
+    except Exception:
+        if not silent:
+            raise
+        logger.warning('unknown error obtained when sanitizing %r', src, exc_info=True)
+        cleaned = '<p>Unknown error when sanitizing</p>'
     return cleaned