mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Avoid sending CSRF token in GET and external forms
Summary: Sending CSRF token in GET forms is dangerous because if there are external links on the target page then the token could leak through Referer header. The token is not required for anything because GET forms are used only to display data, not to perform operations. Sending CSRF tokens to external URLs leaks the token immediately. Please note that <form action> defaults to GET. PhabricatorUserOAuthSettingsPanelController suffered from this problem for both reasons. Test Plan: Save my settings (POST form). Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1558
This commit is contained in:
parent
c0efecb561
commit
6bd8542abb
4 changed files with 30 additions and 31 deletions
|
@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'aphront/request');
|
|||
phutil_require_module('phabricator', 'infrastructure/celerity/api');
|
||||
|
||||
phutil_require_module('phutil', 'markup');
|
||||
phutil_require_module('phutil', 'utils');
|
||||
|
||||
|
||||
phutil_require_source('markup.php');
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* Copyright 2011 Facebook, Inc.
|
||||
* Copyright 2012 Facebook, Inc.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
|
@ -49,22 +49,25 @@ function javelin_render_tag(
|
|||
|
||||
|
||||
function phabricator_render_form(PhabricatorUser $user, $attributes, $content) {
|
||||
return javelin_render_tag(
|
||||
'form',
|
||||
$attributes,
|
||||
phutil_render_tag(
|
||||
'input',
|
||||
array(
|
||||
'type' => 'hidden',
|
||||
'name' => AphrontRequest::getCSRFTokenName(),
|
||||
'value' => $user->getCSRFToken(),
|
||||
)).
|
||||
phutil_render_tag(
|
||||
'input',
|
||||
array(
|
||||
'type' => 'hidden',
|
||||
'name' => '__form__',
|
||||
'value' => true,
|
||||
)).$content);
|
||||
if (strcasecmp(idx($attributes, 'method'), 'POST') == 0 &&
|
||||
!preg_match('#^(https?:|//)#', idx($attributes, 'action'))) {
|
||||
$content =
|
||||
phutil_render_tag(
|
||||
'input',
|
||||
array(
|
||||
'type' => 'hidden',
|
||||
'name' => AphrontRequest::getCSRFTokenName(),
|
||||
'value' => $user->getCSRFToken(),
|
||||
)).
|
||||
phutil_render_tag(
|
||||
'input',
|
||||
array(
|
||||
'type' => 'hidden',
|
||||
'name' => '__form__',
|
||||
'value' => true,
|
||||
)).
|
||||
$content;
|
||||
}
|
||||
return javelin_render_tag('form', $attributes, $content);
|
||||
}
|
||||
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* Copyright 2011 Facebook, Inc.
|
||||
* Copyright 2012 Facebook, Inc.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
|
@ -73,8 +73,12 @@ final class AphrontFormView extends AphrontView {
|
|||
->appendChild($this->renderDataInputs())
|
||||
->appendChild($this->renderChildren());
|
||||
|
||||
return javelin_render_tag(
|
||||
'form',
|
||||
if (!$this->user) {
|
||||
throw new Exception('You must pass the user to AphrontFormView.');
|
||||
}
|
||||
|
||||
return phabricator_render_form(
|
||||
$this->user,
|
||||
array(
|
||||
'action' => $this->action,
|
||||
'method' => $this->method,
|
||||
|
@ -86,16 +90,8 @@ final class AphrontFormView extends AphrontView {
|
|||
}
|
||||
|
||||
private function renderDataInputs() {
|
||||
if (!$this->user) {
|
||||
throw new Exception('You must pass the user to AphrontFormView.');
|
||||
}
|
||||
|
||||
$data = $this->data + array(
|
||||
'__form__' => 1,
|
||||
AphrontRequest::getCSRFTokenName() => $this->user->getCSRFToken(),
|
||||
);
|
||||
$inputs = array();
|
||||
foreach ($data as $key => $value) {
|
||||
foreach ($this->data as $key => $value) {
|
||||
if ($value === null) {
|
||||
continue;
|
||||
}
|
||||
|
|
|
@ -6,7 +6,6 @@
|
|||
|
||||
|
||||
|
||||
phutil_require_module('phabricator', 'aphront/request');
|
||||
phutil_require_module('phabricator', 'infrastructure/celerity/api');
|
||||
phutil_require_module('phabricator', 'infrastructure/javelin/api');
|
||||
phutil_require_module('phabricator', 'infrastructure/javelin/markup');
|
||||
|
|
Loading…
Reference in a new issue