Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix utf8 errors in headers and cookies and invalid parameter errors #2902

Merged
merged 2 commits into from
Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 11 additions & 27 deletions lib/rack_request_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize(request)
end

def values_to_check
param_values + ahoy_headers + ahoy_cookies + [host_header]
param_values + header_values + cookie_values
end

private
Expand All @@ -15,35 +15,19 @@ def param_values
all_values(request.params)
end

def all_values(hash)
hash.values.flat_map { |value| value.is_a?(Hash) ? all_values(value) : [value] }
end

def ahoy_headers
[ahoy_visit_header, ahoy_visitor_header]
end

def ahoy_visit_header
request.fetch_header('HTTP_AHOY_VISIT') { '' }
end

def ahoy_visitor_header
request.fetch_header('HTTP_AHOY_VISITOR') { '' }
end

def ahoy_cookies
[ahoy_visit_cookie, ahoy_visitor_cookie]
def header_values
header_values = []
request.env.each do |key, value|
header_values.push(value) if key.start_with? 'HTTP_'
end
header_values
end

def ahoy_visit_cookie
request.cookies['ahoy_visit']
def cookie_values
request.cookies.values
end

def ahoy_visitor_cookie
request.cookies['ahoy_visitor']
end

def host_header
request.fetch_header('HTTP_HOST') { '' }
def all_values(hash)
hash.values.flat_map { |value| value.is_a?(Hash) ? all_values(value) : [value] }
end
end
29 changes: 23 additions & 6 deletions lib/utf8_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@ def initialize(app)
end

def call(env)
request = Rack::Request.new(env)
parser = RackRequestParser.new(request)
parser = RackRequestParser.new(Rack::Request.new(env))
values_to_check = parser.values_to_check

if invalid_strings(values_to_check)
Rails.logger.info(event_attributes(env, parser.request))
Rails.logger.info(invalid_utf8_event(env, parser.request))
jmhooper marked this conversation as resolved.
Show resolved Hide resolved

return [400, {}, ['Bad request']]
end

@app.call(env)
rescue Rack::QueryParser::InvalidParameterError => err
Rails.logger.info(invalid_parameter_event(err))
[400, {}, ['Bad request']]
end

private
Expand All @@ -34,14 +36,14 @@ def invalid_string?(string)
!string.force_encoding('UTF-8').valid_encoding?
end

def event_attributes(env, request)
def invalid_utf8_event(env, request)
{
event: 'Invalid UTF-8 encoding',
user_uuid: env['warden']&.user&.uuid || AnonymousUser.new.uuid,
ip: remote_ip(request),
user_agent: request.user_agent,
user_agent: sanitized_user_agent(request),
timestamp: Time.zone.now,
hostname: request.host,
hostname: sanitized_hostname(request),
visitor_id: sanitized_visitor_id(request),
content_type: env['CONTENT_TYPE'],
}.to_json
Expand All @@ -51,8 +53,23 @@ def remote_ip(request)
@remote_ip ||= (request.env['action_dispatch.remote_ip'] || request.ip).to_s
end

def sanitized_hostname(request)
Utf8Cleaner.new(request.host).remove_invalid_utf8_bytes
end

def sanitized_user_agent(request)
Utf8Cleaner.new(request.user_agent).remove_invalid_utf8_bytes
end

def sanitized_visitor_id(request)
string_to_clean = request.cookies['ahoy_visitor']
Utf8Cleaner.new(string_to_clean).remove_invalid_utf8_bytes
end

def invalid_parameter_event(error)
{
event: 'Invalid parameter error',
message: error.message,
}
end
end
3 changes: 2 additions & 1 deletion spec/requests/openid_connect_cors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@
describe 'domain name as the origin' do
it 'does not load any ServiceProvider objects' do
stub_const 'ServiceProvider', double
get openid_connect_configuration_path, headers: { 'HTTP_ORIGIN' => Figaro.env.domain_name }
get openid_connect_configuration_path,
headers: { 'HTTP_ORIGIN' => Figaro.env.domain_name.dup }

aggregate_failures do
expect(response).to be_ok
Expand Down