Add latest changes from gitlab-org/security/gitlab@13-5-stable-ee

4 files changed, 48 insertions, 2 deletions

diff --git a/lib/api/internal/kubernetes.rb b/lib/api/internal/kubernetes.rb
index 8175b81f900..90e224b2ccb 100644
--- a/lib/api/internal/kubernetes.rb
+++ b/lib/api/internal/kubernetes.rb
@@ -85,7 +85,7 @@ module API
 
             # TODO sort out authorization for real
             # https://gitlab.com/gitlab-org/gitlab/-/issues/220912
-            if !project || !project.public?
+            unless Ability.allowed?(nil, :download_code, project)
               not_found!
             end
 
diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb
index 7e98f1fc1f7..a6d8a778e05 100644
--- a/lib/gitlab/middleware/multipart.rb
+++ b/lib/gitlab/middleware/multipart.rb
@@ -31,6 +31,7 @@ module Gitlab
       RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS'
       JWT_PARAM_SUFFIX = '.gitlab-workhorse-upload'
       JWT_PARAM_FIXED_KEY = 'upload'
+      REWRITTEN_FIELD_NAME_MAX_LENGTH = 10000.freeze
 
       class Handler
         def initialize(env, message)
@@ -41,6 +42,8 @@ module Gitlab
 
         def with_open_files
           @rewritten_fields.each do |field, tmp_path|
+            raise "invalid field: #{field.inspect}" unless valid_field_name?(field)
+
             parsed_field = Rack::Utils.parse_nested_query(field)
             raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1
 
@@ -108,6 +111,17 @@ module Gitlab
 
         private
 
+        def valid_field_name?(name)
+          # length validation
+          return false if name.size >= REWRITTEN_FIELD_NAME_MAX_LENGTH
+
+          # brackets validation
+          return false if name.include?('[]') || name.start_with?('[', ']')
+          return false unless ::Gitlab::Utils.valid_brackets?(name, allow_nested: false)
+
+          true
+        end
+
         def package_allowed_paths
           packages_config = ::Gitlab.config.packages
           return [] unless allow_packages_storage_path?(packages_config)
@@ -141,6 +155,8 @@ module Gitlab
       class HandlerForJWTParams < Handler
         def with_open_files
           @rewritten_fields.keys.each do |field|
+            raise "invalid field: #{field.inspect}" unless valid_field_name?(field)
+
             parsed_field = Rack::Utils.parse_nested_query(field)
             raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1
 
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 693a10a9de3..8dae77f68f3 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -204,7 +204,7 @@ module Gitlab
     # See https://github.com/docker/distribution/blob/master/reference/regexp.go.
     #
     def container_repository_name_regex
-      @container_repository_regex ||= %r{\A[a-z0-9]+((?:[._/]|__|[-]{0,10})[a-z0-9]+)*\Z}
+      @container_repository_regex ||= %r{\A[a-z0-9]+(([._/]|__|-*)[a-z0-9])*\z}
     end
 
     ##
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index e2d93e7cd29..3df54e74b4f 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -10,6 +10,8 @@ module Gitlab
     # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580
     # It also checks for ALT_SEPARATOR aka '\' (forward slash)
     def check_path_traversal!(path)
+      return unless path.is_a?(String)
+
       path = decode_path(path)
       path_regex = /(\A(\.{1,2})\z|\A\.\.[\/\\]|[\/\\]\.\.\z|[\/\\]\.\.[\/\\]|\n)/
 
@@ -208,5 +210,33 @@ module Gitlab
     def stable_sort_by(list)
       list.sort_by.with_index { |x, idx| [yield(x), idx] }
     end
+
+    # Check for valid brackets (`[` and `]`) in a string using this aspects:
+    # * open brackets count == closed brackets count
+    # * (optionally) reject nested brackets via `allow_nested: false`
+    # * open / close brackets coherence, eg. ][[] -> invalid
+    def valid_brackets?(string = '', allow_nested: true)
+      # remove everything except brackets
+      brackets = string.remove(/[^\[\]]/)
+
+      return true if brackets.empty?
+      # balanced counts check
+      return false if brackets.size.odd?
+
+      unless allow_nested
+        # nested brackets check
+        return false if brackets.include?('[[') || brackets.include?(']]')
+      end
+
+      # open / close brackets coherence check
+      untrimmed = brackets
+      loop do
+        trimmed = untrimmed.gsub('[]', '')
+        return true if trimmed.empty?
+        return false if trimmed == untrimmed
+
+        untrimmed = trimmed
+      end
+    end
   end
 end