1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-16 03:42:41 +01:00
phorge-phorge/src/applications/phame/controller/blog/PhameBlogViewController.php
epriestley 5854de8c1c Don't 302 to an external URI, even after CSRF POST
Summary:
Via HackerOne. This defuses an attack which allows users to steal OAuth tokens through a clever sequence of steps:

  - The attacker begins the OAuth workflow and copies the Facebook URL.
  - The attacker mutates the URL to use the JS/anchor workflow, and to redirect to `/phame/live/X/` instead of `/login/facebook:facebook.com/`, where `X` is the ID of some blog they control. Facebook isn't strict about paths, so this is allowed.
  - The blog has an external domain set (`blog.evil.com`), and the attacker controls that domain.
  - The user gets stopped on the "live" controller with credentials in the page anchor (`#access_token=...`) and a message ("This blog has moved...") in a dialog. They click "Continue", which POSTs a CSRF token.
  - When a user POSTs a `<form />` with no `action` attribute, the browser retains the page anchor. So visiting `/phame/live/8/#anchor` and clicking the "Continue" button POSTs you to a page with `#anchor` intact.
  - Some browsers (including Firefox and Chrome) retain the anchor after a 302 redirect.
  - The OAuth credentials are thus preserved when the user reaches `blog.evil.com`, and the attacker's site can read them.

This 302'ing after CSRF post is unusual in Phabricator and unique to Phame. It's not necessary -- instead, just use normal links, which drop anchors.

I'm going to pursue further steps to mitigate this class of attack more thoroughly:

  - Ideally, we should render forms with an explicit `action` attribute, but this might be a lot of work. I might render them with `#` if no action is provided. We never expect anchors to survive POST, and it's surprising to me that they do.
  - I'm going to blacklist OAuth parameters (like `access_token`) from appearing in GET on all pages except whitelisted pages (login pages). Although it's not important here, I think these could be captured from referrers in some cases. See also T4342.

Test Plan: Browsed all the affected Phame interfaces.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, arice

Differential Revision: https://secure.phabricator.com/D8481
2014-03-10 16:21:07 -07:00

195 lines
5 KiB
PHP

<?php
/**
* @group phame
*/
final class PhameBlogViewController extends PhameController {
private $id;
public function willProcessRequest(array $data) {
$this->id = $data['id'];
}
public function processRequest() {
$request = $this->getRequest();
$user = $request->getUser();
$blog = id(new PhameBlogQuery())
->setViewer($user)
->withIDs(array($this->id))
->executeOne();
if (!$blog) {
return new Aphront404Response();
}
$pager = id(new AphrontCursorPagerView())
->readFromRequest($request);
$posts = id(new PhamePostQuery())
->setViewer($user)
->withBlogPHIDs(array($blog->getPHID()))
->executeWithCursorPager($pager);
$nav = $this->renderSideNavFilterView(null);
$header = id(new PHUIHeaderView())
->setHeader($blog->getName())
->setUser($user)
->setPolicyObject($blog);
$handle_phids = array_merge(
mpull($posts, 'getBloggerPHID'),
mpull($posts, 'getBlogPHID'));
$this->loadHandles($handle_phids);
$actions = $this->renderActions($blog, $user);
$properties = $this->renderProperties($blog, $user, $actions);
$post_list = $this->renderPostList(
$posts,
$user,
pht('This blog has no visible posts.'));
require_celerity_resource('phame-css');
$post_list = id(new PHUIBoxView())
->addPadding(PHUI::PADDING_LARGE)
->addClass('phame-post-list')
->appendChild($post_list);
$crumbs = $this->buildApplicationCrumbs();
$crumbs->addTextCrumb($blog->getName(), $this->getApplicationURI());
$object_box = id(new PHUIObjectBoxView())
->setHeader($header)
->addPropertyList($properties);
$nav->appendChild(
array(
$crumbs,
$object_box,
$post_list,
));
return $this->buildApplicationPage(
$nav,
array(
'device' => true,
'title' => $blog->getName(),
));
}
private function renderProperties(
PhameBlog $blog,
PhabricatorUser $user,
PhabricatorActionListView $actions) {
require_celerity_resource('aphront-tooltip-css');
Javelin::initBehavior('phabricator-tooltips');
$properties = new PHUIPropertyListView();
$properties->setActionList($actions);
$properties->addProperty(
pht('Skin'),
$blog->getSkin());
$properties->addProperty(
pht('Domain'),
$blog->getDomain());
$feed_uri = PhabricatorEnv::getProductionURI(
$this->getApplicationURI('blog/feed/'.$blog->getID().'/'));
$properties->addProperty(
pht('Atom URI'),
javelin_tag('a',
array(
'href' => $feed_uri,
'sigil' => 'has-tooltip',
'meta' => array(
'tip' => pht('Atom URI does not support custom domains.'),
'size' => 320,
)
),
$feed_uri));
$descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
$user,
$blog);
$properties->addProperty(
pht('Editable By'),
$descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
$properties->addProperty(
pht('Joinable By'),
$descriptions[PhabricatorPolicyCapability::CAN_JOIN]);
$engine = id(new PhabricatorMarkupEngine())
->setViewer($user)
->addObject($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION)
->process();
$properties->addTextContent(
phutil_tag(
'div',
array(
'class' => 'phabricator-remarkup',
),
$engine->getOutput($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION)));
return $properties;
}
private function renderActions(PhameBlog $blog, PhabricatorUser $user) {
$actions = id(new PhabricatorActionListView())
->setObject($blog)
->setObjectURI($this->getRequest()->getRequestURI())
->setUser($user);
$can_edit = PhabricatorPolicyFilter::hasCapability(
$user,
$blog,
PhabricatorPolicyCapability::CAN_EDIT);
$can_join = PhabricatorPolicyFilter::hasCapability(
$user,
$blog,
PhabricatorPolicyCapability::CAN_JOIN);
$actions->addAction(
id(new PhabricatorActionView())
->setIcon('new')
->setHref($this->getApplicationURI('post/edit/?blog='.$blog->getID()))
->setName(pht('Write Post'))
->setDisabled(!$can_join)
->setWorkflow(!$can_join));
$actions->addAction(
id(new PhabricatorActionView())
->setUser($user)
->setIcon('world')
->setHref($blog->getLiveURI())
->setName(pht('View Live')));
$actions->addAction(
id(new PhabricatorActionView())
->setIcon('edit')
->setHref($this->getApplicationURI('blog/edit/'.$blog->getID().'/'))
->setName('Edit Blog')
->setDisabled(!$can_edit)
->setWorkflow(!$can_edit));
$actions->addAction(
id(new PhabricatorActionView())
->setIcon('delete')
->setHref($this->getApplicationURI('blog/delete/'.$blog->getID().'/'))
->setName('Delete Blog')
->setDisabled(!$can_edit)
->setWorkflow(true));
return $actions;
}
}