From bcf255e9c96bb267e9b48df0d028e0dcf0ec4544 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 26 Feb 2014 11:17:46 -0800
Subject: [PATCH] Require CSRF submission to verify email addresses

Summary: If an attacker somehow intercepts a verification URL for an email address, they can hypothetically CSRF the account owner into verifying it. What you'd do before (how do you get the link?) and after (why do you care that you tricked them into verifying) performing this attack is unclear, but in theory we should require a CSRF submission here; add one.

Test Plan: {F118691}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8351
---
 ...PhabricatorEmailVerificationController.php | 40 ++++++++++++-------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorEmailVerificationController.php b/src/applications/auth/controller/PhabricatorEmailVerificationController.php
index 0dbb918851..bca3f27f20 100644
--- a/src/applications/auth/controller/PhabricatorEmailVerificationController.php
+++ b/src/applications/auth/controller/PhabricatorEmailVerificationController.php
@@ -36,6 +36,8 @@ final class PhabricatorEmailVerificationController
       $user->getPHID(),
       $this->code);
 
+    $submit = null;
+
     if (!$email) {
       $title = pht('Unable to Verify Email');
       $content = pht(
@@ -49,38 +51,46 @@ final class PhabricatorEmailVerificationController
       $content = pht(
         'This email address has already been verified.');
       $continue = pht('Continue to Phabricator');
-    } else {
-      $guard = AphrontWriteGuard::beginScopedUnguardedWrites();
-        $email->openTransaction();
+    } else if ($request->isFormPost()) {
+      $email->openTransaction();
 
-          $email->setIsVerified(1);
-          $email->save();
+        $email->setIsVerified(1);
+        $email->save();
 
-          // If the user just verified their primary email address, mark their
-          // account as email verified.
-          $user_primary = $user->loadPrimaryEmail();
-          if ($user_primary->getID() == $email->getID()) {
-            $user->setIsEmailVerified(1);
-            $user->save();
-          }
+        // If the user just verified their primary email address, mark their
+        // account as email verified.
+        $user_primary = $user->loadPrimaryEmail();
+        if ($user_primary->getID() == $email->getID()) {
+          $user->setIsEmailVerified(1);
+          $user->save();
+        }
 
-        $email->saveTransaction();
-      unset($guard);
+      $email->saveTransaction();
 
       $title = pht('Address Verified');
       $content = pht(
         'The email address %s is now verified.',
         phutil_tag('strong', array(), $email->getAddress()));
       $continue = pht('Continue to Phabricator');
+    } else {
+      $title = pht('Verify Email Address');
+      $content = pht(
+        'Verify this email address (%s) and attach it to your account?',
+        phutil_tag('strong', array(), $email->getAddress()));
+      $continue = pht('Cancel');
+      $submit = pht('Verify %s', $email->getAddress());
     }
 
     $dialog = id(new AphrontDialogView())
       ->setUser($user)
       ->setTitle($title)
-      ->setMethod('GET')
       ->addCancelButton('/', $continue)
       ->appendChild($content);
 
+    if ($submit) {
+      $dialog->addSubmitButton($submit);
+    }
+
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb(pht('Verify Email'));
 
-- 
2.51.2