diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 462c8f3e7df..a36efcf8e2f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,7 @@ +require 'core_extensions/string/permit' + class ApplicationController < ActionController::Base + String.include CoreExtensions::String::Permit include UserSessionContext include VerifyProfileConcern include LocaleHelper diff --git a/app/controllers/sign_up/email_resend_controller.rb b/app/controllers/sign_up/email_resend_controller.rb index 5a46b03636d..2ee962a5af0 100644 --- a/app/controllers/sign_up/email_resend_controller.rb +++ b/app/controllers/sign_up/email_resend_controller.rb @@ -1,7 +1,5 @@ module SignUp class EmailResendController < ApplicationController - include UnconfirmedUserConcern - def new @user = User.new @resend_email_confirmation_form = ResendEmailConfirmationForm.new diff --git a/lib/core_extensions/string/permit.rb b/lib/core_extensions/string/permit.rb new file mode 100644 index 00000000000..37fb341ebd4 --- /dev/null +++ b/lib/core_extensions/string/permit.rb @@ -0,0 +1,18 @@ +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +module CoreExtensions + module String + module Permit + def permit(*) + raise ActionController::ParameterMissing, '#permit called on String' + end + end + end +end diff --git a/spec/requests/invalid_sign_in_params_spec.rb b/spec/requests/invalid_sign_in_params_spec.rb index c4721221143..b42a0196ea2 100644 --- a/spec/requests/invalid_sign_in_params_spec.rb +++ b/spec/requests/invalid_sign_in_params_spec.rb @@ -1,8 +1,12 @@ require 'rails_helper' describe 'visiting sign in page with invalid user params' do - it 'does not raise an exception' do - get new_user_session_path, params: { user: 'test@test.com' } + it 'raises ActionController::ParameterMissing' do + params = { user: 'test@test.com' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { get new_user_session_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) end end diff --git a/spec/requests/params_is_string_instead_of_hash_spec.rb b/spec/requests/params_is_string_instead_of_hash_spec.rb new file mode 100644 index 00000000000..8c265907ce8 --- /dev/null +++ b/spec/requests/params_is_string_instead_of_hash_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +describe 'submitting email resend form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { resend_email_confirmation_form: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_create_email_resend_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end + +describe 'submitting email registration form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { user: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_register_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end