Commit d04dc6d2 authored by Kassio Borges's avatar Kassio Borges

Deny localhost requests on fogbugz importer

Fogbugz importer controller is not validating the URI param received to
do the import. This is a blind SSRF security fail. To fix it a
validation on the URI param was added not allowing the import to perform
when receiving localhost URIs.
parent 68e796e3
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Import::FogbugzController < Import::BaseController class Import::FogbugzController < Import::BaseController
before_action :verify_fogbugz_import_enabled before_action :verify_fogbugz_import_enabled
before_action :user_map, only: [:new_user_map, :create_user_map] before_action :user_map, only: [:new_user_map, :create_user_map]
before_action :verify_blocked_uri, only: :callback
rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized
...@@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController ...@@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController
def verify_fogbugz_import_enabled def verify_fogbugz_import_enabled
render_404 unless fogbugz_import_enabled? render_404 unless fogbugz_import_enabled?
end end
def verify_blocked_uri
Gitlab::UrlBlocker.validate!(
params[:uri],
{
allow_localhost: allow_local_requests?,
allow_local_network: allow_local_requests?,
schemes: %w(http https)
}
)
rescue Gitlab::UrlBlocker::BlockedUrlError => e
redirect_to new_import_fogbugz_url, alert: _('Specified URL cannot be used: "%{reason}"') % { reason: e.message }
end
def allow_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end end
---
title: Deny localhost requests on fogbugz importer
merge_request:
author:
type: security
...@@ -18773,6 +18773,9 @@ msgstr "" ...@@ -18773,6 +18773,9 @@ msgstr ""
msgid "Specified URL cannot be used." msgid "Specified URL cannot be used."
msgstr "" msgstr ""
msgid "Specified URL cannot be used: \"%{reason}\""
msgstr ""
msgid "Specify an e-mail address regex pattern to identify default internal users." msgid "Specify an e-mail address regex pattern to identify default internal users."
msgstr "" msgstr ""
......
...@@ -25,6 +25,35 @@ describe Import::FogbugzController do ...@@ -25,6 +25,35 @@ describe Import::FogbugzController do
expect(session[:fogbugz_uri]).to eq(uri) expect(session[:fogbugz_uri]).to eq(uri)
expect(response).to redirect_to(new_user_map_import_fogbugz_path) expect(response).to redirect_to(new_user_map_import_fogbugz_path)
end end
context 'verify url' do
shared_examples 'denies local request' do |reason|
it 'does not allow requests' do
post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' }
expect(response).to redirect_to(new_import_fogbugz_url)
expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{reason}\"")
end
end
context 'when host is localhost' do
let(:uri) { 'https://localhost:3000' }
include_examples 'denies local request', 'Requests to localhost are not allowed'
end
context 'when host is on local network' do
let(:uri) { 'http://192.168.0.1/' }
include_examples 'denies local request', 'Requests to the local network are not allowed'
end
context 'when host is ftp protocol' do
let(:uri) { 'ftp://testing' }
include_examples 'denies local request', 'Only allowed schemes are http, https'
end
end
end end
describe 'POST #create_user_map' do describe 'POST #create_user_map' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment