1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-19 16:58:48 +02:00

Include OAuth targets in "form-action" Content-Security-Policy

Summary:
Ref T4340. Some "Register/Login" and "Link External Account" buttons are forms which submit to third-party sites. Whitelist these targets when pages render an OAuth form.

Safari, at least, also prevents a redirect to a third-party domain after a form submission to the local domain, so when we first redirect locally (as with Twitter and other OAuth1 providers) we need to authorize an additional URI.

Test Plan: Clicked all my registration buttons locally without hitting CSP issues.

Maniphest Tasks: T4340

Differential Revision: https://secure.phabricator.com/D19159
This commit is contained in:
epriestley 2018-02-28 19:07:19 -08:00
parent d5befb1a0e
commit 94d340fcff
6 changed files with 29 additions and 14 deletions

View file

@ -24,9 +24,10 @@ abstract class AphrontResponse extends Phobject {
final public function addContentSecurityPolicyURI($kind, $uri) {
if ($this->contentSecurityPolicyURIs === null) {
$this->contentSecurityPolicyURIs = array(
'script' => array(),
'connect' => array(),
'frame' => array(),
'script-src' => array(),
'connect-src' => array(),
'frame-src' => array(),
'form-action' => array(),
);
}
@ -125,14 +126,14 @@ abstract class AphrontResponse extends Phobject {
// On a small number of pages, including the Stripe workflow and the
// ReCAPTCHA challenge, we embed external Javascript directly.
$csp[] = $this->newContentSecurityPolicy('script', $default);
$csp[] = $this->newContentSecurityPolicy('script-src', $default);
// We need to specify that we can connect to ourself in order for AJAX
// requests to work.
$csp[] = $this->newContentSecurityPolicy('connect', "'self'");
$csp[] = $this->newContentSecurityPolicy('connect-src', "'self'");
// DarkConsole and PHPAST both use frames to render some content.
$csp[] = $this->newContentSecurityPolicy('frame', "'self'");
$csp[] = $this->newContentSecurityPolicy('frame-src', "'self'");
// This is a more modern flavor of of "X-Frame-Options" and prevents
// clickjacking attacks where the page is included in a tiny iframe and
@ -152,7 +153,7 @@ abstract class AphrontResponse extends Phobject {
// This can result in some trickiness with file downloads if applications
// try to start downloads by submitting a dialog. Redirect to the file's
// download URI instead of submitting a form to it.
$csp[] = "form-action 'self'";
$csp[] = $this->newContentSecurityPolicy('form-action', "'self'");
// Block use of "<base>" to change the origin of relative URIs on the page.
$csp[] = "base-uri 'none'";
@ -177,7 +178,7 @@ abstract class AphrontResponse extends Phobject {
}
$sources = array_unique($sources);
return "{$type}-src ".implode(' ', $sources);
return $type.' '.implode(' ', $sources);
}
private function newContentSecurityPolicySource($uri) {

View file

@ -447,6 +447,13 @@ abstract class PhabricatorAuthProvider extends Phobject {
));
}
$static_response = CelerityAPI::getStaticResourceResponse();
$static_response->addContentSecurityPolicyURI('form-action', (string)$uri);
foreach ($this->getContentSecurityPolicyFormActions() as $csp_uri) {
$static_response->addContentSecurityPolicyURI('form-action', $csp_uri);
}
return phabricator_form(
$viewer,
array(
@ -505,4 +512,8 @@ abstract class PhabricatorAuthProvider extends Phobject {
throw new PhutilMethodNotImplementedException();
}
protected function getContentSecurityPolicyFormActions() {
return array();
}
}

View file

@ -208,6 +208,9 @@ abstract class PhabricatorOAuth1AuthProvider
parent::willRenderLinkedAccount($viewer, $item, $account);
}
protected function getContentSecurityPolicyFormActions() {
return $this->getAdapter()->getContentSecurityPolicyFormActions();
}
/* -( Temporary Secrets )-------------------------------------------------- */

View file

@ -285,8 +285,8 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
->addScript($src);
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('script', $src)
->addContentSecurityPolicyURI('frame', $src);
->addContentSecurityPolicyURI('script-src', $src)
->addContentSecurityPolicyURI('frame-src', $src);
Javelin::initBehavior(
'stripe-payment-form',

View file

@ -43,9 +43,9 @@ final class AphrontFormRecaptchaControl extends AphrontFormControl {
$pubkey = PhabricatorEnv::getEnvConfig('recaptcha.public-key');
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('script', $js)
->addContentSecurityPolicyURI('script', 'https://www.gstatic.com/')
->addContentSecurityPolicyURI('frame', 'https://www.google.com/');
->addContentSecurityPolicyURI('script-src', $js)
->addContentSecurityPolicyURI('script-src', 'https://www.gstatic.com/')
->addContentSecurityPolicyURI('frame-src', 'https://www.google.com/');
return array(
phutil_tag(

View file

@ -584,7 +584,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
) + $this->buildAphlictListenConfigData());
CelerityAPI::getStaticResourceResponse()
->addContentSecurityPolicyURI('connect', $client_uri);
->addContentSecurityPolicyURI('connect-src', $client_uri);
}
}