From 358ec89c03c3068993a1b73499d2e59f9d977088 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Mon, 19 Sep 2016 17:06:17 +0200 Subject: [PATCH] Apply security hotfix 20160830 for redirects. --- CHANGES.rst | 2 +- plone/app/discussion/browser/moderation.py | 8 ++- .../discussion/tests/test_moderation_view.py | 50 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 9a0748d..894e27f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,7 +14,7 @@ New features: Bug fixes: -- *add item here* +- Apply security hotfix 20160830 for redirects. [maurits] - Update Traditional Chinese translation. [l34marr] diff --git a/plone/app/discussion/browser/moderation.py b/plone/app/discussion/browser/moderation.py index 722386d..f1623ea 100644 --- a/plone/app/discussion/browser/moderation.py +++ b/plone/app/discussion/browser/moderation.py @@ -105,7 +105,9 @@ class DeleteComment(BrowserView): type='info') came_from = self.context.REQUEST.HTTP_REFERER # if the referrer already has a came_from in it, don't redirect back - if len(came_from) == 0 or 'came_from=' in came_from: + if (len(came_from) == 0 or 'came_from=' in came_from or + not getToolByName( + content_object, 'portal_url').isURLInPortal(came_from)): came_from = content_object.absolute_url() return self.context.REQUEST.RESPONSE.redirect(came_from) @@ -186,7 +188,9 @@ class PublishComment(BrowserView): type='info') came_from = self.context.REQUEST.HTTP_REFERER # if the referrer already has a came_from in it, don't redirect back - if len(came_from) == 0 or 'came_from=' in came_from: + if (len(came_from) == 0 or 'came_from=' in came_from or + not getToolByName( + content_object, 'portal_url').isURLInPortal(came_from)): came_from = content_object.absolute_url() return self.context.REQUEST.RESPONSE.redirect(came_from) diff --git a/plone/app/discussion/tests/test_moderation_view.py b/plone/app/discussion/tests/test_moderation_view.py index 9233a8c..849a0d0 100644 --- a/plone/app/discussion/tests/test_moderation_view.py +++ b/plone/app/discussion/tests/test_moderation_view.py @@ -1,12 +1,17 @@ # -*- coding: utf-8 -*- from plone.app.discussion.browser.moderation import BulkActionsView +from plone.app.discussion.browser.moderation import DeleteComment +from plone.app.discussion.browser.moderation import PublishComment from plone.app.discussion.browser.moderation import View from plone.app.discussion.interfaces import IConversation +from plone.app.discussion.interfaces import IDiscussionSettings from plone.app.discussion.testing import PLONE_APP_DISCUSSION_INTEGRATION_TESTING # noqa from plone.app.testing import setRoles from plone.app.testing import TEST_USER_ID +from plone.registry.interfaces import IRegistry from Products.CMFCore.utils import getToolByName from zope.component import createObject +from zope.component import queryUtility import unittest @@ -155,3 +160,48 @@ class ModerationBulkActionsViewTest(unittest.TestCase): comment = self.conversation.getComments().next() self.assertTrue(comment) self.assertEqual(comment, self.comment2) + + +class RedirectionTest(unittest.TestCase): + + layer = PLONE_APP_DISCUSSION_INTEGRATION_TESTING + + def setUp(self): + # Update settings. + self.portal = self.layer['portal'] + self.request = self.layer['request'] + setRoles(self.portal, TEST_USER_ID, ['Manager']) + # applyProfile(self.portal, 'plone.app.discussion:default') + registry = queryUtility(IRegistry) + settings = registry.forInterface(IDiscussionSettings) + settings.globally_enabled = True + self.portal.portal_workflow.setChainForPortalTypes( + ('Discussion Item',), + ('comment_review_workflow',)) + # Create page plus comment. + self.portal.invokeFactory( + id='page', + title='Page 1', + type_name='Document' + ) + self.page = self.portal.page + self.conversation = IConversation(self.page) + comment = createObject('plone.Comment') + comment.text = 'Comment text' + self.comment_id = self.conversation.addComment(comment) + self.comment = list(self.conversation.getComments())[0] + + def test_regression(self): + page_url = self.page.absolute_url() + self.request['HTTP_REFERER'] = page_url + for Klass in (DeleteComment, PublishComment): + view = Klass(self.comment, self.request) + view.__parent__ = self.comment + self.assertEqual(page_url, view()) + + def test_valid_next_url(self): + self.request['HTTP_REFERER'] = 'http://attacker.com' + for Klass in (DeleteComment, PublishComment): + view = Klass(self.comment, self.request) + view.__parent__ = self.comment + self.assertNotEqual('http://attacker.com', view())