summaryrefslogtreecommitdiff
path: root/lib/gitlab/middleware
diff options
context:
space:
mode:
Diffstat (limited to 'lib/gitlab/middleware')
-rw-r--r--lib/gitlab/middleware/handle_malformed_strings.rb103
-rw-r--r--lib/gitlab/middleware/handle_null_bytes.rb61
-rw-r--r--lib/gitlab/middleware/read_only/controller.rb39
3 files changed, 121 insertions, 82 deletions
diff --git a/lib/gitlab/middleware/handle_malformed_strings.rb b/lib/gitlab/middleware/handle_malformed_strings.rb
new file mode 100644
index 00000000000..84f7e2e1b14
--- /dev/null
+++ b/lib/gitlab/middleware/handle_malformed_strings.rb
@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Middleware
+ # There is no valid reason for a request to contain a malformed string
+ # so just return HTTP 400 (Bad Request) if we receive one
+ class HandleMalformedStrings
+ include ActionController::HttpAuthentication::Basic
+
+ NULL_BYTE_REGEX = Regexp.new(Regexp.escape("\u0000")).freeze
+
+ attr_reader :app
+
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ return [400, { 'Content-Type' => 'text/plain' }, ['Bad Request']] if request_contains_malformed_string?(env)
+
+ app.call(env)
+ end
+
+ private
+
+ def request_contains_malformed_string?(env)
+ return false if ENV['DISABLE_REQUEST_VALIDATION'] == '1'
+
+ # Duplicate the env, so it is not modified when accessing the parameters
+ # https://github.com/rails/rails/blob/34991a6ae2fc68347c01ea7382fa89004159e019/actionpack/lib/action_dispatch/http/parameters.rb#L59
+ # The modification causes problems with our multipart middleware
+ request = ActionDispatch::Request.new(env.dup)
+
+ return true if malformed_path?(request.path)
+ return true if credentials_malformed?(request)
+
+ request.params.values.any? do |value|
+ param_has_null_byte?(value)
+ end
+ rescue ActionController::BadRequest
+ # If we can't build an ActionDispatch::Request something's wrong
+ # This would also happen if `#params` contains invalid UTF-8
+ # in this case we'll return a 400
+ #
+ true
+ end
+
+ def malformed_path?(path)
+ string_malformed?(Rack::Utils.unescape(path))
+ rescue ArgumentError
+ # Rack::Utils.unescape raised this, path is malformed.
+ true
+ end
+
+ def credentials_malformed?(request)
+ credentials = if has_basic_credentials?(request)
+ decode_credentials(request).presence
+ else
+ request.authorization.presence
+ end
+
+ return false unless credentials
+
+ string_malformed?(credentials)
+ end
+
+ def param_has_null_byte?(value, depth = 0)
+ # Guard against possible attack sending large amounts of nested params
+ # Should be safe as deeply nested params are highly uncommon.
+ return false if depth > 2
+
+ depth += 1
+
+ if value.respond_to?(:match)
+ string_malformed?(value)
+ elsif value.respond_to?(:values)
+ value.values.any? do |hash_value|
+ param_has_null_byte?(hash_value, depth)
+ end
+ elsif value.is_a?(Array)
+ value.any? do |array_value|
+ param_has_null_byte?(array_value, depth)
+ end
+ else
+ false
+ end
+ end
+
+ def string_malformed?(string)
+ # We're using match rather than include, because that will raise an ArgumentError
+ # when the string contains invalid UTF8
+ #
+ # We try to encode the string from ASCII-8BIT to UTF8. If we failed to do
+ # so for certain characters in the string, those chars are probably incomplete
+ # multibyte characters.
+ string.encode(Encoding::UTF_8).match?(NULL_BYTE_REGEX)
+ rescue ArgumentError, Encoding::UndefinedConversionError
+ # If we're here, we caught a malformed string. Return true
+ true
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/middleware/handle_null_bytes.rb b/lib/gitlab/middleware/handle_null_bytes.rb
deleted file mode 100644
index c88dfb6ee0b..00000000000
--- a/lib/gitlab/middleware/handle_null_bytes.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module Middleware
- # There is no valid reason for a request to contain a null byte (U+0000)
- # so just return HTTP 400 (Bad Request) if we receive one
- class HandleNullBytes
- NULL_BYTE_REGEX = Regexp.new(Regexp.escape("\u0000")).freeze
-
- attr_reader :app
-
- def initialize(app)
- @app = app
- end
-
- def call(env)
- return [400, {}, ["Bad Request"]] if request_has_null_byte?(env)
-
- app.call(env)
- end
-
- private
-
- def request_has_null_byte?(request)
- return false if ENV['REJECT_NULL_BYTES'] == "1"
-
- request = Rack::Request.new(request)
-
- request.params.values.any? do |value|
- param_has_null_byte?(value)
- end
- end
-
- def param_has_null_byte?(value, depth = 0)
- # Guard against possible attack sending large amounts of nested params
- # Should be safe as deeply nested params are highly uncommon.
- return false if depth > 2
-
- depth += 1
-
- if value.respond_to?(:match)
- string_contains_null_byte?(value)
- elsif value.respond_to?(:values)
- value.values.any? do |hash_value|
- param_has_null_byte?(hash_value, depth)
- end
- elsif value.is_a?(Array)
- value.any? do |array_value|
- param_has_null_byte?(array_value, depth)
- end
- else
- false
- end
- end
-
- def string_contains_null_byte?(string)
- string.match?(NULL_BYTE_REGEX)
- end
- end
- end
-end
diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb
index cfea4aaddf3..101172cdfcc 100644
--- a/lib/gitlab/middleware/read_only/controller.rb
+++ b/lib/gitlab/middleware/read_only/controller.rb
@@ -9,20 +9,19 @@ module Gitlab
APPLICATION_JSON_TYPES = %W{#{APPLICATION_JSON} application/vnd.git-lfs+json}.freeze
ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
- WHITELISTED_GIT_ROUTES = {
- 'repositories/git_http' => %w{git_upload_pack git_receive_pack}
+ ALLOWLISTED_GIT_ROUTES = {
+ 'repositories/git_http' => %w{git_upload_pack}
}.freeze
- WHITELISTED_GIT_LFS_ROUTES = {
- 'repositories/lfs_api' => %w{batch},
- 'repositories/lfs_locks_api' => %w{verify create unlock}
+ ALLOWLISTED_GIT_LFS_BATCH_ROUTES = {
+ 'repositories/lfs_api' => %w{batch}
}.freeze
- WHITELISTED_GIT_REVISION_ROUTES = {
+ ALLOWLISTED_GIT_REVISION_ROUTES = {
'projects/compare' => %w{create}
}.freeze
- WHITELISTED_SESSION_ROUTES = {
+ ALLOWLISTED_SESSION_ROUTES = {
'sessions' => %w{destroy},
'admin/sessions' => %w{create destroy}
}.freeze
@@ -55,7 +54,7 @@ module Gitlab
def disallowed_request?
DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) &&
- !whitelisted_routes
+ !allowlisted_routes
end
def json_request?
@@ -87,8 +86,8 @@ module Gitlab
end
# Overridden in EE module
- def whitelisted_routes
- workhorse_passthrough_route? || internal_route? || lfs_route? || compare_git_revisions_route? || sidekiq_route? || session_route? || graphql_query?
+ def allowlisted_routes
+ workhorse_passthrough_route? || internal_route? || lfs_batch_route? || compare_git_revisions_route? || sidekiq_route? || session_route? || graphql_query?
end
# URL for requests passed through gitlab-workhorse to rails-web
@@ -96,9 +95,9 @@ module Gitlab
def workhorse_passthrough_route?
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.post? &&
- request.path.end_with?('.git/git-upload-pack', '.git/git-receive-pack')
+ request.path.end_with?('.git/git-upload-pack')
- WHITELISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+ ALLOWLISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def internal_route?
@@ -109,18 +108,16 @@ module Gitlab
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.post? && request.path.end_with?('compare')
- WHITELISTED_GIT_REVISION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+ ALLOWLISTED_GIT_REVISION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
- def lfs_route?
+ # Batch upload requests are blocked in:
+ # https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/repositories/lfs_api_controller.rb#L106
+ def lfs_batch_route?
# Calling route_hash may be expensive. Only do it if we think there's a possible match
- unless request.path.end_with?('/info/lfs/objects/batch',
- '/info/lfs/locks', '/info/lfs/locks/verify') ||
- %r{/info/lfs/locks/\d+/unlock\z}.match?(request.path)
- return false
- end
+ return unless request.path.end_with?('/info/lfs/objects/batch')
- WHITELISTED_GIT_LFS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+ ALLOWLISTED_GIT_LFS_BATCH_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def session_route?
@@ -128,7 +125,7 @@ module Gitlab
return false unless request.post? && request.path.end_with?('/users/sign_out',
'/admin/session', '/admin/session/destroy')
- WHITELISTED_SESSION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+ ALLOWLISTED_SESSION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def sidekiq_route?
View Lorry Controller Status