From 15518852aa3b07e2fa36703e474de6e38691d55b Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 4 Mar 2020 22:08:07 -0800 Subject: [PATCH] Added validation to reject email addresses over 250 characters --- .../Auth/ForgotPasswordController.php | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Auth/ForgotPasswordController.php b/app/Http/Controllers/Auth/ForgotPasswordController.php index c64710ac38..9a0a3d879a 100644 --- a/app/Http/Controllers/Auth/ForgotPasswordController.php +++ b/app/Http/Controllers/Auth/ForgotPasswordController.php @@ -5,6 +5,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use Illuminate\Foundation\Auth\SendsPasswordResetEmails; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Validator; class ForgotPasswordController extends Controller { @@ -41,6 +42,8 @@ class ForgotPasswordController extends Controller return property_exists($this, 'subject') ? $this->subject : \Lang::get('mail.reset_link'); } + + /** * Send a reset link to the given user. * @@ -49,11 +52,21 @@ class ForgotPasswordController extends Controller */ public function sendResetLinkEmail(Request $request) { - $this->validate($request, ['email' => 'required|email']); - // We will send the password reset link to this user. Once we have attempted - // to send the link, we will examine the response then see the message we - // need to show to the user. Finally, we'll send out a proper response. + /** + * Let's set a max character count here to prevent potential + * buffer overflow issues with attackers sending very large + * payloads through. + */ + $this->validate($request, ['email' => 'required|email|max:250']); + + /** + * If we find a matching email with an activated yser, we will + * send the password reset link to the user. + * + * Once we have attempted to send the link, we will examine the response + * then see the message we need to show to the user. Finally, we'll send out a proper response. + */ $response = $this->broker()->sendResetLink( array_merge( $request->only('email'), @@ -65,9 +78,25 @@ class ForgotPasswordController extends Controller return redirect()->route('login')->with('status', trans($response)); } - // If an error was returned by the password broker, we will get this message - // translated so we can notify a user of the problem. We'll redirect back - // to where the users came from so they can attempt this process again. + + /** + * If an error was returned by the password broker, we will get this message + * translated so we can notify a user of the problem. We'll redirect back + * to where the users came from so they can attempt this process again. + * + * HOWEVER, we do not want to translate the message if the user isn't found + * or isn't active, since that would allow an attacker to walk through + * a dictionary attack and figure out registered user email addresses. + * + * Instead we tell the user we've sent an email even though we haven't. + * It's bad UX, but better security. The compromises we sometimes have to make. + */ + + if ($response == 'passwords.user') { + \Log::debug('User with email '.$request->input('email').' attempted a password reset request but was not found. No email was sent.'); + return redirect()->route('login')->with('success', trans('passwords.user_inactive')); + } + return back()->withErrors( ['email' => trans($response)] );