[FIX] fields.html, forum: opt-in stripping of @style attrs
For public-facing HTML content provided by the user, `<style>` tags and `style` attributes should be stripped automatically, as they can easily be abused to deface pages for abusive users and spammers. <style> tags were already stripped, the optional `strip_style` for fields.html enables the automatic stripping of style attributes. This is opt-in because custom style attributes are still desirable in trusted HTML fields.
This commit is contained in:
parent
948befbb34
commit
13476c844d
|
@ -252,7 +252,7 @@ class Post(osv.Model):
|
||||||
_columns = {
|
_columns = {
|
||||||
'name': fields.char('Title'),
|
'name': fields.char('Title'),
|
||||||
'forum_id': fields.many2one('forum.forum', 'Forum', required=True),
|
'forum_id': fields.many2one('forum.forum', 'Forum', required=True),
|
||||||
'content': fields.html('Content'),
|
'content': fields.html('Content', strip_style=True),
|
||||||
'tag_ids': fields.many2many('forum.tag', 'forum_tag_rel', 'forum_id', 'forum_tag_id', 'Tags'),
|
'tag_ids': fields.many2many('forum.tag', 'forum_tag_rel', 'forum_id', 'forum_tag_id', 'Tags'),
|
||||||
'state': fields.selection([('active', 'Active'), ('close', 'Close'), ('offensive', 'Offensive')], 'Status'),
|
'state': fields.selection([('active', 'Active'), ('close', 'Close'), ('offensive', 'Offensive')], 'Status'),
|
||||||
'views': fields.integer('Number of Views'),
|
'views': fields.integer('Number of Views'),
|
||||||
|
|
|
@ -14,7 +14,8 @@ class WebsiteResPartner(osv.Model):
|
||||||
'website_published': fields.boolean(
|
'website_published': fields.boolean(
|
||||||
'Publish', help="Publish on the website", copy=False),
|
'Publish', help="Publish on the website", copy=False),
|
||||||
'website_description': fields.html(
|
'website_description': fields.html(
|
||||||
'Website Partner Full Description'
|
'Website Partner Full Description',
|
||||||
|
strip_style=True
|
||||||
),
|
),
|
||||||
'website_short_description': fields.text(
|
'website_short_description': fields.text(
|
||||||
'Website Partner Short Description'
|
'Website Partner Short Description'
|
||||||
|
|
|
@ -146,7 +146,7 @@ class TestHtmlField(common.TransactionCase):
|
||||||
% if object.some_field and not object.oriented:
|
% if object.some_field and not object.oriented:
|
||||||
<table>
|
<table>
|
||||||
% if object.other_field:
|
% if object.other_field:
|
||||||
<tr>
|
<tr style="border: 10px solid black;">
|
||||||
${object.mako_thing}
|
${object.mako_thing}
|
||||||
<td>
|
<td>
|
||||||
</tr>
|
</tr>
|
||||||
|
@ -173,5 +173,11 @@ class TestHtmlField(common.TransactionCase):
|
||||||
# sanitize should have closed tags left open in the original html
|
# sanitize should have closed tags left open in the original html
|
||||||
self.assertIn('</table>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
self.assertIn('</table>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
||||||
self.assertIn('</td>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
self.assertIn('</td>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
||||||
|
self.assertIn('<tr style="', partner.comment, 'Style attr should not have been stripped')
|
||||||
|
|
||||||
|
self.partner._columns['comment'] = fields.html('Stripped Html', sanitize=True, strip_style=True)
|
||||||
|
self.partner.write(cr, uid, [pid], {'comment': some_ugly_html}, context=context)
|
||||||
|
partner = self.partner.browse(cr, uid, pid, context=context)
|
||||||
|
self.assertNotIn('<tr style="', partner.comment, 'Style attr should have been stripped')
|
||||||
|
|
||||||
self.partner._columns = old_columns
|
self.partner._columns = old_columns
|
||||||
|
|
|
@ -1076,16 +1076,21 @@ class Text(_String):
|
||||||
class Html(_String):
|
class Html(_String):
|
||||||
type = 'html'
|
type = 'html'
|
||||||
sanitize = True # whether value must be sanitized
|
sanitize = True # whether value must be sanitized
|
||||||
|
strip_style = False # whether to strip style attributes
|
||||||
|
|
||||||
_column_sanitize = property(attrgetter('sanitize'))
|
_column_sanitize = property(attrgetter('sanitize'))
|
||||||
_related_sanitize = property(attrgetter('sanitize'))
|
_related_sanitize = property(attrgetter('sanitize'))
|
||||||
_description_sanitize = property(attrgetter('sanitize'))
|
_description_sanitize = property(attrgetter('sanitize'))
|
||||||
|
|
||||||
|
_column_strip_style = property(attrgetter('strip_style'))
|
||||||
|
_related_strip_style = property(attrgetter('strip_style'))
|
||||||
|
_description_strip_style = property(attrgetter('strip_style'))
|
||||||
|
|
||||||
def convert_to_cache(self, value, record, validate=True):
|
def convert_to_cache(self, value, record, validate=True):
|
||||||
if value is None or value is False:
|
if value is None or value is False:
|
||||||
return False
|
return False
|
||||||
if validate and self.sanitize:
|
if validate and self.sanitize:
|
||||||
return html_sanitize(value)
|
return html_sanitize(value, strip_style=self.strip_style)
|
||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -317,11 +317,12 @@ class html(text):
|
||||||
return None
|
return None
|
||||||
if not self._sanitize:
|
if not self._sanitize:
|
||||||
return value
|
return value
|
||||||
return html_sanitize(value)
|
return html_sanitize(value, strip_style=self._strip_style)
|
||||||
|
|
||||||
def __init__(self, string='unknown', sanitize=True, **args):
|
def __init__(self, string='unknown', sanitize=True, strip_style=False, **args):
|
||||||
super(html, self).__init__(string=string, **args)
|
super(html, self).__init__(string=string, **args)
|
||||||
self._sanitize = sanitize
|
self._sanitize = sanitize
|
||||||
|
self._strip_style = strip_style
|
||||||
# symbol_set redefinition because of sanitize specific behavior
|
# symbol_set redefinition because of sanitize specific behavior
|
||||||
self._symbol_f = self._symbol_set_html
|
self._symbol_f = self._symbol_set_html
|
||||||
self._symbol_set = (self._symbol_c, self._symbol_f)
|
self._symbol_set = (self._symbol_c, self._symbol_f)
|
||||||
|
|
|
@ -53,7 +53,7 @@ safe_attrs = clean.defs.safe_attrs | frozenset(
|
||||||
])
|
])
|
||||||
|
|
||||||
|
|
||||||
def html_sanitize(src, silent=True, strict=False):
|
def html_sanitize(src, silent=True, strict=False, strip_style=False):
|
||||||
if not src:
|
if not src:
|
||||||
return src
|
return src
|
||||||
src = ustr(src, errors='replace')
|
src = ustr(src, errors='replace')
|
||||||
|
@ -69,7 +69,7 @@ def html_sanitize(src, silent=True, strict=False):
|
||||||
|
|
||||||
kwargs = {
|
kwargs = {
|
||||||
'page_structure': True,
|
'page_structure': True,
|
||||||
'style': False, # do not remove style attributes
|
'style': strip_style, # True = remove style tags/attrs
|
||||||
'forms': True, # remove form tags
|
'forms': True, # remove form tags
|
||||||
'remove_unknown_tags': False,
|
'remove_unknown_tags': False,
|
||||||
'allow_tags': allowed_tags,
|
'allow_tags': allowed_tags,
|
||||||
|
|
Loading…
Reference in New Issue