From 2f6613846489509d7aafb4d292e0b52bd497fb90 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 19 Feb 2013 16:04:54 -0800 Subject: [PATCH] Fix an open redirect issue in Phame with "View Live" Summary: Currently, you can set a blog URI to "evil.com" and then the live controller will issue a redirect. Instead, require a CSRF check. If it fails, pop a "this blog has moved" dialog. Test Plan: - Clicked "View Live" for in-app and on-domain blogs and posts. - Hit URI directly. {F33302} Reviewers: vrana Reviewed By: vrana CC: cbg, aran Differential Revision: https://secure.phabricator.com/D5021 --- .../blog/PhameBlogLiveController.php | 19 +++++++++++++++++-- .../blog/PhameBlogViewController.php | 2 ++ .../post/PhamePostViewController.php | 2 ++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/applications/phame/controller/blog/PhameBlogLiveController.php b/src/applications/phame/controller/blog/PhameBlogLiveController.php index 40f0bfe3bd..9b3e1fec80 100644 --- a/src/applications/phame/controller/blog/PhameBlogLiveController.php +++ b/src/applications/phame/controller/blog/PhameBlogLiveController.php @@ -30,8 +30,23 @@ final class PhameBlogLiveController extends PhameController { } if ($blog->getDomain() && ($request->getHost() != $blog->getDomain())) { - return id(new AphrontRedirectResponse()) - ->setURI('http://'.$blog->getDomain().'/'.$this->more); + $base_uri = 'http://'.$blog->getDomain().'/'; + if ($request->isFormPost()) { + return id(new AphrontRedirectResponse()) + ->setURI($base_uri.$this->more); + } else { + // If we don't have CSRF, return a dialog instead of automatically + // redirecting, to prevent this endpoint from serving semi-open + // redirects. + $dialog = id(new AphrontDialogView()) + ->setTitle(pht('Blog Moved')) + ->setUser($user) + ->appendChild( + pht('This blog is now hosted at %s.', + $base_uri)) + ->addSubmitButton(pht('Continue')); + return id(new AphrontDialogResponse())->setDialog($dialog); + } } $phame_request = clone $request; diff --git a/src/applications/phame/controller/blog/PhameBlogViewController.php b/src/applications/phame/controller/blog/PhameBlogViewController.php index 00214cf100..8accd39a1c 100644 --- a/src/applications/phame/controller/blog/PhameBlogViewController.php +++ b/src/applications/phame/controller/blog/PhameBlogViewController.php @@ -133,8 +133,10 @@ final class PhameBlogViewController extends PhameController { $actions->addAction( id(new PhabricatorActionView()) + ->setUser($user) ->setIcon('world') ->setHref($this->getApplicationURI('live/'.$blog->getID().'/')) + ->setRenderAsForm(true) ->setName(pht('View Live'))); $actions->addAction( diff --git a/src/applications/phame/controller/post/PhamePostViewController.php b/src/applications/phame/controller/post/PhamePostViewController.php index d29b582a05..d7edfeb172 100644 --- a/src/applications/phame/controller/post/PhamePostViewController.php +++ b/src/applications/phame/controller/post/PhamePostViewController.php @@ -139,9 +139,11 @@ final class PhamePostViewController extends PhameController { $actions->addAction( id(new PhabricatorActionView()) + ->setUser($user) ->setIcon('world') ->setHref($live_uri) ->setName(pht('View Live')) + ->setRenderAsForm(true) ->setDisabled(!$can_view_live) ->setWorkflow(!$can_view_live));