Commit 49d90ade authored by Stan Hu's avatar Stan Hu Committed by David Fernandez

Fix filename bypass when uploading NuGet packages

Validate the incoming workhorse file.
parent 3b1fbe09
---
title: Fix filename bypass when uploading NuGet packages
merge_request:
author:
type: security
...@@ -78,13 +78,13 @@ module API ...@@ -78,13 +78,13 @@ module API
detail 'This feature was introduced in GitLab 12.6' detail 'This feature was introduced in GitLab 12.6'
end end
params do params do
use :workhorse_upload_params requires :package, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
end end
put do put do
authorize_upload!(authorized_user_project) authorize_upload!(authorized_user_project)
file_params = params.merge( file_params = params.merge(
file: uploaded_package_file(:package), file: params[:package],
file_name: PACKAGE_FILENAME file_name: PACKAGE_FILENAME
) )
......
...@@ -130,14 +130,17 @@ describe API::NugetPackages do ...@@ -130,14 +130,17 @@ describe API::NugetPackages do
let(:url) { "/projects/#{project.id}/packages/nuget" } let(:url) { "/projects/#{project.id}/packages/nuget" }
let(:headers) { {} } let(:headers) { {} }
let(:params) { { package: temp_file(file_name) } } let(:params) { { package: temp_file(file_name) } }
let(:file_key) { :package }
let(:send_rewritten_field) { true }
subject do subject do
workhorse_finalize( workhorse_finalize(
api(url), api(url),
method: :put, method: :put,
file_key: :package, file_key: file_key,
params: params, params: params,
headers: headers headers: headers,
send_rewritten_field: send_rewritten_field
) )
end end
......
...@@ -171,7 +171,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member = ...@@ -171,7 +171,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
end end
context 'without a file from workhorse' do context 'without a file from workhorse' do
let(:params) { { package: nil } } let(:send_rewritten_field) { false }
it_behaves_like 'returning response status', :bad_request it_behaves_like 'returning response status', :bad_request
end end
...@@ -212,6 +212,19 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member = ...@@ -212,6 +212,19 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
it_behaves_like 'returning response status', :forbidden it_behaves_like 'returning response status', :forbidden
end end
end end
context 'with crafted package.path param' do
let(:crafted_file) { Tempfile.new('nuget.crafted.package.path') }
let(:url) { "/projects/#{project.id}/packages/nuget?package.path=#{crafted_file.path}" }
let(:params) { { file: temp_file(file_name) } }
let(:file_key) { :file }
it 'does not create a package file' do
expect { subject }.to change { ::Packages::PackageFile.count }.by(0)
end
it_behaves_like 'returning response status', :bad_request
end
end end
context 'and direct upload disabled' do context 'and direct upload disabled' 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