From a2515921b635cb0d997068223c0cd1234be165cf Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 23 Jan 2014 14:03:28 -0800 Subject: [PATCH] Detect developer error when constructing forms with absolute URIs Summary: Ref T1921. Ref T4339. If you `phabricator_form()` with an absolute URI, we silently drop the CSRF tokens. This can be confusing if you meant to specify `"/some/path"` but ended up specifying `"http://this.install.com/some/path"`. In all current cases that I can think of / am aware of, this indicates an error in the code. Make it more obvious what's happening and how to fix it. The error only fires in developer mode. Test Plan: Hit this case, also rendered normal forms. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4339, T1921 Differential Revision: https://secure.phabricator.com/D8044 --- src/infrastructure/javelin/markup.php | 57 +++++++++++++++++++-------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/infrastructure/javelin/markup.php b/src/infrastructure/javelin/markup.php index 5477f5687b..b845b33ae8 100644 --- a/src/infrastructure/javelin/markup.php +++ b/src/infrastructure/javelin/markup.php @@ -38,23 +38,48 @@ function javelin_tag( function phabricator_form(PhabricatorUser $user, $attributes, $content) { $body = array(); - if (strcasecmp(idx($attributes, 'method'), 'POST') == 0 && - !preg_match('#^(https?:|//)#', idx($attributes, 'action'))) { - $body[] = phutil_tag( - 'input', - array( - 'type' => 'hidden', - 'name' => AphrontRequest::getCSRFTokenName(), - 'value' => $user->getCSRFToken(), - )); + $http_method = idx($attributes, 'method'); + $is_post = (strcasecmp($http_method, 'POST') === 0); - $body[] = phutil_tag( - 'input', - array( - 'type' => 'hidden', - 'name' => '__form__', - 'value' => true, - )); + $http_action = idx($attributes, 'action'); + $is_absolute_uri = preg_match('#^(https?:|//)#', $http_action); + + if ($is_post) { + if ($is_absolute_uri) { + $is_dev = PhabricatorEnv::getEnvConfig('phabricator.developer-mode'); + if ($is_dev) { + $form_domain = id(new PhutilURI($http_action)) + ->getDomain(); + $host_domain = id(new PhutilURI(PhabricatorEnv::getURI('/'))) + ->getDomain(); + + if (strtolower($form_domain) == strtolower($host_domain)) { + throw new Exception( + pht( + "You are building a
that submits to Phabricator, but ". + "has an absolute URI in its 'action' attribute ('%s'). To avoid ". + "leaking CSRF tokens, Phabricator does not add CSRF information ". + "to forms with absolute URIs. Instead, use a relative URI.", + $http_action)); + } + } + } else { + $body[] = phutil_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => AphrontRequest::getCSRFTokenName(), + 'value' => $user->getCSRFToken(), + )); + + $body[] = phutil_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => '__form__', + 'value' => true, + )); + } } if (is_array($content)) {