Add latest changes from gitlab-org/security/gitlab@16-0-stable-eev16.0.1

9 files changed, 86 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 70a33628b03..785f22815ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 16.0.1 (2023-05-22)
+
+### Security (1 change)
+
+- [Fix arbitary file read via filename param](gitlab-org/security/gitlab@2ddbf5464954addce7b8c82102377f0f137b604f) ([merge request](gitlab-org/security/gitlab!3265))
+
 ## 16.0.0 (2023-05-18)
 
 ### Added (168 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 0e94e314c86..ab54c33c763 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-16.0.0
\ No newline at end of file
+16.0.1
\ No newline at end of file
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION
index 0e94e314c86..ab54c33c763 100644
--- a/GITLAB_PAGES_VERSION
+++ b/GITLAB_PAGES_VERSION
@@ -1 +1 @@
-16.0.0
\ No newline at end of file
+16.0.1
\ No newline at end of file
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index db756ae336f..0d64a685065 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -10,6 +10,10 @@ module UploadsActions
   included do
     prepend_before_action :set_request_format_from_path_extension
     rescue_from FileUploader::InvalidSecret, with: :render_404
+
+    rescue_from ::Gitlab::Utils::PathTraversalAttackError do
+      head :bad_request
+    end
   end
 
   def create
@@ -33,6 +37,8 @@ module UploadsActions
   #   - or redirect to its URL
   #
   def show
+    Gitlab::Utils.check_path_traversal!(params[:filename])
+
     return render_404 unless uploader&.exists?
 
     ttl, directives = *cache_settings
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index 4188e0caa8e..0a30f0e99f7 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -410,6 +410,8 @@ module ObjectStorage
     end
 
     def retrieve_from_store!(identifier)
+      Gitlab::Utils.check_path_traversal!(identifier)
+
       # We need to force assign the value of @filename so that we will still
       # get the original_filename in cases wherein the file points to a random generated
       # path format. This happens for direct uploaded files to final location.
diff --git a/spec/requests/groups/uploads_controller_spec.rb b/spec/requests/groups/uploads_controller_spec.rb
new file mode 100644
index 00000000000..8a9e3edbe20
--- /dev/null
+++ b/spec/requests/groups/uploads_controller_spec.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::UploadsController, feature_category: :shared do
+  include WorkhorseHelpers
+
+  it_behaves_like 'uploads actions' do
+    let_it_be(:model) { create(:group, :public) }
+    let_it_be(:upload) { create(:upload, :namespace_upload, :with_file, model: model) }
+
+    let(:show_path) { show_group_uploads_path(model, upload.secret, File.basename(upload.path)) }
+  end
+end
diff --git a/spec/requests/projects/uploads_controller_spec.rb b/spec/requests/projects/uploads_controller_spec.rb
new file mode 100644
index 00000000000..03b53143c84
--- /dev/null
+++ b/spec/requests/projects/uploads_controller_spec.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::UploadsController, feature_category: :shared do
+  include WorkhorseHelpers
+
+  it_behaves_like 'uploads actions' do
+    let_it_be(:model) { create(:project, :public) }
+    let_it_be(:upload) { create(:upload, :issuable_upload, :with_file, model: model) }
+
+    let(:show_path) { show_project_uploads_path(model, upload.secret, File.basename(upload.path)) }
+  end
+end
diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb
new file mode 100644
index 00000000000..1e6999982a8
--- /dev/null
+++ b/spec/requests/uploads_controller_spec.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe UploadsController, feature_category: :shared do
+  include WorkhorseHelpers
+
+  it_behaves_like 'uploads actions' do
+    let_it_be(:model) { create(:personal_snippet, :public) }
+    let_it_be(:upload) { create(:upload, :personal_snippet_upload, :with_file, model: model) }
+
+    # See config/routes/uploads.rb
+    let(:show_path) do
+      "/uploads/-/system/#{model.model_name.singular}/#{model.to_param}/#{upload.secret}/#{File.basename(upload.path)}"
+    end
+  end
+end
diff --git a/spec/support/shared_examples/requests/uploads_actions_shared_examples.rb b/spec/support/shared_examples/requests/uploads_actions_shared_examples.rb
new file mode 100644
index 00000000000..41613fa9871
--- /dev/null
+++ b/spec/support/shared_examples/requests/uploads_actions_shared_examples.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'uploads actions' do
+  describe "GET #show" do
+    context 'with file traversal in filename parameter' do
+      # Uploads in tests are stored in directories like:
+      # tmp/tests/public/uploads/@hashed/AB/CD/ABCD/SECRET
+      let(:filename) { "../../../../../../../../../Gemfile.lock" }
+      let(:escaped_filename) { CGI.escape filename }
+
+      it 'responds with status 400' do
+        # Check files do indeed exists
+        upload_absolute_path = Pathname(upload.absolute_path)
+        expect(upload_absolute_path).to be_exist
+        attacked_file_path = upload_absolute_path.dirname.join(filename)
+        expect(attacked_file_path).to be_exist
+
+        # Need to escape, otherwise we get `ActionController::UrlGenerationError Exception: No route matches`
+        get show_path.sub(File.basename(upload.path), escaped_filename)
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+    end
+  end
+end