Commit c93ce836 authored by manojmj's avatar manojmj

Do not allow localhost url redirection in GitHub Integration

parent d1154dcd
---
title: Do not allow localhost url redirection in GitHub Integration
merge_request:
author:
type: security
Octokit.middleware.insert_after Octokit::Middleware::FollowRedirects, Gitlab::Octokit::Middleware
...@@ -40,7 +40,7 @@ module Gitlab ...@@ -40,7 +40,7 @@ module Gitlab
# otherwise hitting the rate limit will result in a thread # otherwise hitting the rate limit will result in a thread
# being blocked in a `sleep()` call for up to an hour. # being blocked in a `sleep()` call for up to an hour.
def initialize(token, per_page: 100, parallel: true) def initialize(token, per_page: 100, parallel: true)
@octokit = Octokit::Client.new( @octokit = ::Octokit::Client.new(
access_token: token, access_token: token,
per_page: per_page, per_page: per_page,
api_endpoint: api_endpoint api_endpoint: api_endpoint
...@@ -139,7 +139,7 @@ module Gitlab ...@@ -139,7 +139,7 @@ module Gitlab
begin begin
yield yield
rescue Octokit::TooManyRequests rescue ::Octokit::TooManyRequests
raise_or_wait_for_rate_limit raise_or_wait_for_rate_limit
# This retry will only happen when running in sequential mode as we'll # This retry will only happen when running in sequential mode as we'll
......
...@@ -101,7 +101,7 @@ module Gitlab ...@@ -101,7 +101,7 @@ module Gitlab
# GitHub Rate Limit API returns 404 when the rate limit is # GitHub Rate Limit API returns 404 when the rate limit is
# disabled. In this case we just want to return gracefully # disabled. In this case we just want to return gracefully
# instead of spitting out an error. # instead of spitting out an error.
rescue Octokit::NotFound rescue ::Octokit::NotFound
nil nil
end end
......
# frozen_string_literal: true
module Gitlab
module Octokit
class Middleware
def initialize(app)
@app = app
end
def call(env)
Gitlab::UrlBlocker.validate!(env[:url], { allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests? })
@app.call(env)
end
private
def allow_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
end
end
end
end
require 'spec_helper'
describe Gitlab::Octokit::Middleware do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
shared_examples 'Public URL' do
it 'does not raise an error' do
expect(app).to receive(:call).with(env)
expect { middleware.call(env) }.not_to raise_error
end
end
shared_examples 'Local URL' do
it 'raises an error' do
expect { middleware.call(env) }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
end
end
describe '#call' do
context 'when the URL is a public URL' do
let(:env) { { url: 'https://public-url.com' } }
it_behaves_like 'Public URL'
end
context 'when the URL is a localhost adresss' do
let(:env) { { url: 'http://127.0.0.1' } }
context 'when localhost requests are not allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: false)
end
it_behaves_like 'Local URL'
end
context 'when localhost requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
end
it_behaves_like 'Public URL'
end
end
context 'when the URL is a local network address' do
let(:env) { { url: 'http://172.16.0.0' } }
context 'when local network requests are not allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: false)
end
it_behaves_like 'Local URL'
end
context 'when local network requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
end
it_behaves_like 'Public URL'
end
end
end
end
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