Skip to content

Commit

Permalink
Eliminate XSS vulnerability in field help field
Browse files Browse the repository at this point in the history
  • Loading branch information
smcmahon committed May 1, 2016
1 parent fc9592b commit b9590e5
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 7 deletions.
8 changes: 5 additions & 3 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
Change History
==============


1.8.1 (unreleased)
1.8.1 (2016-05-01)
------------------

- Nothing changed yet.
- CGI escape field help (description); prevent method call.
Eliminates XSS vulnerability that could be exploited by users with the ability
to create forms.
[smcmahon]


1.8.0 (2015-10-01)
Expand Down
38 changes: 38 additions & 0 deletions Products/PloneFormGen/content/field_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import cgi
from Products.Archetypes.Renderer import renderer


class ATWidgetWrapper(object):
"""
Wrap a Products.Archetypes.generator.widget.widget class
to intercept the Description method.
"""

def __init__(self, obj):
self.obj = obj

def __call__(self, mode, instance, context=None):
return self.obj(mode, instance, context)

def __getattr__(self, name):
if name == 'Description':
return self.wDescription
return getattr(self.obj, name)

def wDescription(self, instance, **kwargs):
value = self.obj.description
if value:
return cgi.escape(value)
else:
return value


def widget(self, field_name, mode="view", field=None, **kwargs):
"""Returns the rendered widget.
"""

if field is None:
field = self.Schema()[field_name]
widget = ATWidgetWrapper(field.widget)
return renderer.render(field_name, mode, widget, self, field=field,
**kwargs)
6 changes: 6 additions & 0 deletions Products/PloneFormGen/content/fieldsBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

from Products.PloneFormGen import PloneFormGenMessageFactory as _

import field_utils


def finalizeFieldSchema(schema, folderish=True, moveDiscussion=False):
""" cleanup typical field schema """
Expand Down Expand Up @@ -953,3 +955,7 @@ def manage_afterAdd(self, item, container):
id = self.getId()
if self.fgField.__name__ != id:
self.fgField.__name__ = id


def widget(self, field_name, mode="view", field=None, **kwargs):
return field_utils.widget(self, field_name, mode, field, **kwargs)
8 changes: 7 additions & 1 deletion Products/PloneFormGen/content/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
from plone.registry.interfaces import IRegistry
from zope.component import getUtility

import field_utils

from types import StringTypes

import zope.i18n
Expand Down Expand Up @@ -1053,7 +1055,7 @@ def reorderField(self, item_id, target_id, insert_method, **kw):
""" move item to target"""

plone.protect.CheckAuthenticator(self.REQUEST)

itemPos = self.getObjectPosition(item_id)
targetPos = self.getObjectPosition(target_id)

Expand Down Expand Up @@ -1112,5 +1114,9 @@ def lastFieldIdFromForm(self, **kw):
lastField = myFields[-1].id
return lastField

security.declareProtected(View, 'widget')
def widget(self, field_name, mode="view", field=None, **kwargs):
return field_utils.widget(self, field_name, mode, field, **kwargs)


registerATCT(FormFolder, PROJECTNAME)
6 changes: 4 additions & 2 deletions Products/PloneFormGen/skins/PloneFormGen/fg_base_view_p3.cpt
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@
<metal:use_body use-macro="body_macro" />

<tal:if tal:condition="here/getFormEpilogue | nothing">
<metal:field use-macro="python:context.widget('formEpilogue', mode='view')">
<div style="clear:both">
<metal:field use-macro="python:context.widget('formEpilogue', mode='view')">
Body text
</metal:field>
</metal:field>
</div>
</tal:if>

</tal:def_macro>
Expand Down
51 changes: 51 additions & 0 deletions Products/PloneFormGen/tests/browser.txt
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,57 @@ Attempts to read our special member attributes TTW should fail::
...
HTTPError: HTTP Error 404: Not Found

Archetypes does not quote an input widget's description. That's fine for Archetypes,
where descriptions are set in code, but it's not OK when form help may be
set by untrusted users.
Check for quoting.

>>> browser.open(portal_url + '/testform')
>>> browser.getLink('String Field').click()
>>> browser.getControl('Field Label').value = 'stringtest'
>>> browser.getControl('Field Help').value = '<img id="fhelp" />'
>>> browser.getControl('Save').click()

Check Form Field display

>>> browser.open(portal_url + '/testform/stringtest')
>>> '<img id="fhelp" />' in browser.contents
False
>>> '&lt;img id="fhelp" /&gt;' in browser.contents
True

Check Form display

>>> browser.open(portal_url + '/testform')
>>> '<img id="fhelp" />' in browser.contents
False
>>> '&lt;img id="fhelp" /&gt;' in browser.contents
True

Archetypes also allows a widget description field to contain the name of
a method, which it will call to get the description value. We must not do that.

>>> browser.open(portal_url + '/testform/stringtest/edit')
>>> browser.getControl('Field Help').value = 'absolute_url'
>>> browser.getControl('Save').click()

Check Form Field

>>> browser.open(portal_url + '/testform/stringtest')
>>> '<span class="formHelp" id="stringtest_help">http' in browser.contents
False
>>> '<span class="formHelp" id="stringtest_help">absolute_url' in browser.contents
True

Check Form display

>>> browser.open(portal_url + '/testform')
>>> '<span class="formHelp" id="stringtest_help">http' in browser.contents
False
>>> '<span class="formHelp" id="stringtest_help">absolute_url' in browser.contents
True



# Embedded form
# -------------
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from setuptools import setup, find_packages

version = '1.8.0'
version = '1.8.1'

setup(name='Products.PloneFormGen',
version=version,
Expand Down

0 comments on commit b9590e5

Please sign in to comment.