You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2013/01/16 21:35:08 UTC
[1/5] git commit: [#5596] Replaced most occurances of HTMLField with
safer DisplayOnlyWidget
[#5596] Replaced most occurances of HTMLField with safer DisplayOnlyWidget
Signed-off-by: Cory Johns <jo...@geek.net>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/16477571
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/16477571
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/16477571
Branch: refs/heads/master
Commit: 16477571d93e7e81f61d051503de433dd219d3a4
Parents: db9be9d
Author: Cory Johns <jo...@geek.net>
Authored: Mon Jan 14 23:55:28 2013 +0000
Committer: Cory Johns <jo...@geek.net>
Committed: Mon Jan 14 23:55:28 2013 +0000
----------------------------------------------------------------------
Allura/allura/controllers/auth.py | 4 +-
Allura/allura/lib/widgets/discuss.py | 5 +-
Allura/allura/lib/widgets/form_fields.py | 31 +++++++-------
Allura/allura/lib/widgets/forms.py | 32 ++++++---------
Allura/allura/lib/widgets/subscriptions.py | 13 +++---
Allura/allura/model/discuss.py | 2 +
.../forgediscussion/widgets/forum_widgets.py | 17 +++++---
7 files changed, 55 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/controllers/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 0ef414a..9f8bf8d 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -6,6 +6,8 @@ from tg import expose, session, flash, redirect, validate, config
from tg.decorators import with_trailing_slash
from pylons import c, g, request, response
from webob import exc as wexc
+from ew import jinja2_ew as ew
+from jinja2.filters import escape as j2_escape
import allura.tasks.repo_tasks
from allura import model as M
@@ -397,7 +399,7 @@ class PreferencesController(BaseController):
continue
title = mb.artifact_title
if mb.artifact_url:
- title = '<a href="%s">%s</a>' % (mb.artifact_url,title)
+ title = ew.LinkField(label=j2_escape(mb.artifact_title), href=j2_escape(mb.artifact_url)).display()
subscriptions.append(dict(
subscription_id=mb._id,
project_name=project.name,
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/lib/widgets/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/discuss.py b/Allura/allura/lib/widgets/discuss.py
index c34bfa9..ff38529 100644
--- a/Allura/allura/lib/widgets/discuss.py
+++ b/Allura/allura/lib/widgets/discuss.py
@@ -166,8 +166,9 @@ class _ThreadsTable(ew.TableField):
class hidden_fields(ew_core.NameList):
_id=ew.HiddenField(validator=V.Ming(M.Thread))
class fields(ew_core.NameList):
- num_replies=ew.HTMLField(show_label=True, label='Num Posts')
- num_views=ew.HTMLField(show_label=True)
+ num_replies=ffw.DisplayOnlyField(show_label=True, label='Num Posts')
+ num_views=ffw.DisplayOnlyField(show_label=True)
+ # XXX XSS this use of HTMLField is potentially insecure, as value.summary() doesn't properly escape its data
last_post=ew.HTMLField(text="${value and value.summary()}", show_label=True)
subscription=ew.Checkbox(suppress_label=True, show_label=True)
fields.insert(0, ew.LinkField(
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/lib/widgets/form_fields.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/form_fields.py b/Allura/allura/lib/widgets/form_fields.py
index 6427143..f205965 100644
--- a/Allura/allura/lib/widgets/form_fields.py
+++ b/Allura/allura/lib/widgets/form_fields.py
@@ -435,21 +435,22 @@ class Lightbox(ew_core.Widget):
''' % (self.name, self.trigger))
-class LabeledHiddenField(ew.HiddenField):
+class DisplayOnlyField(ew.HiddenField):
'''
- Jinja2 implementation of InputField seems to ignore show_label=True.
-
- FIXME: This should be fixed in EasyWidgets and this class removed.
+ Render a field as plain text, optionally with a hidden field to preserve the value.
'''
- template=ew.Snippet('''<label>{{ label|e }}<input {{widget.j2_attrs({
- 'type':'hidden',
- 'name':name,
- 'class':css_class,
- 'value':value}, attrs)}}></label>''', 'jinja2')
-
-class LabelOnlyField(ew.InputField):
- template=ew.Snippet('<label>{{ label|e }}</label>', 'jinja2')
+ template=ew.Snippet('''{{ (text or value or attrs.value)|e }}
+ {%- if with_hidden_input is none and name or with_hidden_input -%}
+ <input {{
+ widget.j2_attrs({
+ 'type':'hidden',
+ 'name':name,
+ 'value':value,
+ 'class':css_class}, attrs)
+ }}>
+ {%- endif %}''', 'jinja2')
defaults=dict(
- name=None,
- label='',
- show_errors=False)
+ ew.HiddenField.defaults,
+ text=None,
+ value=None,
+ with_hidden_input=None)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/lib/widgets/forms.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index c2c6702..ee4311c 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -258,8 +258,8 @@ class RemoveTextValueForm(ForgeForm):
ew.RowField(
show_errors=False,
fields=[
- ffw.LabelOnlyField(label=label),
- ffw.LabeledHiddenField(
+ ffw.DisplayOnlyField(text=label),
+ ffw.DisplayOnlyField(
name='oldvalue',
label=initial_value,
attrs={'value':initial_value},
@@ -307,13 +307,12 @@ class RemoveSocialNetworkForm(ForgeForm):
ew.RowField(
show_errors=False,
fields=[
- ffw.LabeledHiddenField(
- label='%s account' % socialnetwork,
+ ffw.DisplayOnlyField(
+ text='%s account' % socialnetwork,
name="socialnetwork",
attrs={'value':socialnetwork},
show_errors=False),
- ffw.LabeledHiddenField(
- label=account,
+ ffw.DisplayOnlyField(
name="account",
attrs={'value':account},
show_errors=False),
@@ -364,13 +363,11 @@ class RemoveInactivePeriodForm(ForgeForm):
show_label=False,
show_errors=False,
fields=[
- ffw.LabeledHiddenField(
- label=startdate.strftime('%d/%m/%Y'),
+ ffw.DisplayOnlyField(
name='startdate',
attrs={'value':startdate.strftime('%d/%m/%Y')},
show_errors=False),
- ffw.LabeledHiddenField(
- label=enddate.strftime('%d/%m/%Y'),
+ ffw.DisplayOnlyField(
name='enddate',
attrs={'value':enddate.strftime('%d/%m/%Y')},
show_errors=False),
@@ -429,18 +426,15 @@ class RemoveTimeSlotForm(ForgeForm):
show_errors=False,
show_label=False,
fields=[
- ffw.LabeledHiddenField(
- label=weekday,
+ ffw.DisplayOnlyField(
name='weekday',
attrs={'value':weekday},
show_errors=False),
- ffw.LabeledHiddenField(
- label=starttime.strftime('%H:%M'),
+ ffw.DisplayOnlyField(
name='starttime',
attrs={'value':starttime.strftime('%H:%M')},
show_errors=False),
- ffw.LabeledHiddenField(
- label=endtime.strftime('%H:%M'),
+ ffw.DisplayOnlyField(
name='endtime',
attrs={'value':endtime.strftime('%H:%M')},
show_errors=False),
@@ -579,9 +573,9 @@ class RemoveSkillForm(ForgeForm):
show_errors=False)
],
fields=[
- ffw.LabelOnlyField(label=skill['skill'].fullpath),
- ffw.LabelOnlyField(label=skill['level']),
- ffw.LabelOnlyField(label=comment),
+ ffw.DisplayOnlyField(text=skill['skill'].fullpath),
+ ffw.DisplayOnlyField(text=skill['level']),
+ ffw.DisplayOnlyField(text=comment),
ew.SubmitButton(
show_label=False,
attrs={'value':'Remove'},
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/lib/widgets/subscriptions.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/subscriptions.py b/Allura/allura/lib/widgets/subscriptions.py
index b92b255..e9efa00 100644
--- a/Allura/allura/lib/widgets/subscriptions.py
+++ b/Allura/allura/lib/widgets/subscriptions.py
@@ -4,6 +4,7 @@ import ew as ew_core
import ew.jinja2_ew as ew
from allura.lib import validators as V
+from allura.lib.widgets import form_fields as ffw
from allura import model as M
from .form_fields import SubmitButton
@@ -17,12 +18,12 @@ class _SubscriptionTable(ew.TableField):
topic = ew.HiddenField()
artifact_index_id = ew.HiddenField()
class fields(ew_core.NameList):
- project_name = ew.HTMLField(label='Project', show_label=True)
- mount_point = ew.HTMLField(label='App', show_label=True)
- topic = ew.HTMLField(label='Topic', show_label=True)
- type = ew.HTMLField(label='Type', show_label=True)
- frequency = ew.HTMLField(label='Frequency', show_label=True)
- artifact_title = ew.HTMLField(label='Artifact', show_label=True)
+ project_name = ffw.DisplayOnlyField(label='Project', show_label=True, with_hidden_input=False)
+ mount_point = ffw.DisplayOnlyField(label='App', show_label=True, with_hidden_input=False)
+ topic = ffw.DisplayOnlyField(label='Topic', show_label=True, with_hidden_input=False)
+ type = ffw.DisplayOnlyField(label='Type', show_label=True, with_hidden_input=False)
+ frequency = ffw.DisplayOnlyField(label='Frequency', show_label=True, with_hidden_input=False)
+ artifact_title = ffw.DisplayOnlyField(label='Artifact', show_label=True, with_hidden_input=False)
# unsubscribe = SubmitButton()
subscribed = ew.Checkbox(suppress_label=True)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/Allura/allura/model/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py
index f29c425..41c44ce 100644
--- a/Allura/allura/model/discuss.py
+++ b/Allura/allura/model/discuss.py
@@ -503,6 +503,8 @@ class Post(Message, VersionedArtifact, ActivityObject):
return self.thread.primary()
def summary(self):
+ # XXX XSS security hole here: display_name can be manipulated to
+ # contain unescaped HTML, opening a potential XSS attack
return '<a href="%s">%s</a> %s' % (
self.author().url(), self.author().get_pref('display_name'),
h.ago(self.timestamp))
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/16477571/ForgeDiscussion/forgediscussion/widgets/forum_widgets.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/widgets/forum_widgets.py b/ForgeDiscussion/forgediscussion/widgets/forum_widgets.py
index 7410456..8a2f778 100644
--- a/ForgeDiscussion/forgediscussion/widgets/forum_widgets.py
+++ b/ForgeDiscussion/forgediscussion/widgets/forum_widgets.py
@@ -24,8 +24,9 @@ class _ForumsTable(ew.TableField):
class hidden_fields(ew_core.NameList):
_id=ew.HiddenField(validator=V.Ming(M.ForumThread))
class fields(ew_core.NameList):
- num_topics=ew.HTMLField(show_label=True, label='Topics')
- num_posts=ew.HTMLField(show_label=True, label='Posts')
+ num_topics=ffw.DisplayOnlyField(show_label=True, label='Topics')
+ num_posts=ffw.DisplayOnlyField(show_label=True, label='Posts')
+ # XXX XSS this use of HTMLField is potentially insecure, as value.summary() doesn't properly escape its data
last_post=ew.HTMLField(text="${value and value.summary()}",
show_label=True)
subscribed=ew.Checkbox(suppress_label=True, show_label=True)
@@ -39,9 +40,11 @@ class ForumSubscriptionForm(ew.SimpleForm):
class _ThreadsTable(DW._ThreadsTable):
class fields(ew_core.NameList):
- num_replies=ew.HTMLField(show_label=True, label='Num Replies')
- num_views=ew.HTMLField(show_label=True)
+ num_replies=ffw.DisplayOnlyField(show_label=True, label='Num Replies')
+ num_views=ffw.DisplayOnlyField(show_label=True)
+ # XXX XSS this use of HTMLField is potentially insecure, but I'm not sure what values flags can take
flags=ew.HTMLField(show_label=True, text="${unicode(', '.join(value))}")
+ # XXX XSS this use of HTMLField is potentially insecure, as value.summary() doesn't properly escape its data
last_post=ew.HTMLField(text="${value and value.summary()}", show_label=True)
subscription=ew.Checkbox(suppress_label=True, show_label=True)
fields.insert(0, ew.LinkField(
@@ -57,9 +60,11 @@ class ThreadSubscriptionForm(DW.SubscriptionForm):
class AnnouncementsTable(DW._ThreadsTable):
class fields(ew_core.NameList):
- num_replies=ew.HTMLField(show_label=True, label='Num Replies')
- num_views=ew.HTMLField(show_label=True)
+ num_replies=ffw.DisplayOnlyField(show_label=True, label='Num Replies')
+ num_views=ffw.DisplayOnlyField(show_label=True)
+ # XXX XSS this use of HTMLField is potentially insecure, but I'm not sure what values flags can take
flags=ew.HTMLField(show_label=True, text="${unicode(', '.join(value))}")
+ # XXX XSS this use of HTMLField is potentially insecure, as value.summary() doesn't properly escape its data
last_post=ew.HTMLField(text="${value and value.summary()}", show_label=True)
fields.insert(0, ew.LinkField(
label='Subject', text="${value['subject']}",