Commit c5fac895 authored by Igor Drozdov's avatar Igor Drozdov

Return 404 if model id wasn't passed to UploadsController

From the security perspective, we incorrectly return 200 when
id wasn't passed. That allows Workhorse to process the file
that is being uploaded. It can cause massive exhaustion.

Changelog: security
parent 7546a09e
...@@ -36,14 +36,10 @@ class UploadsController < ApplicationController ...@@ -36,14 +36,10 @@ class UploadsController < ApplicationController
end end
def find_model def find_model
return unless params[:id]
upload_model_class.find(params[:id]) upload_model_class.find(params[:id])
end end
def authorize_access! def authorize_access!
return unless model
authorized = authorized =
case model case model
when Note when Note
...@@ -68,8 +64,6 @@ class UploadsController < ApplicationController ...@@ -68,8 +64,6 @@ class UploadsController < ApplicationController
end end
def authorize_create_access! def authorize_create_access!
return unless model
authorized = authorized =
case model case model
when User when User
......
...@@ -666,6 +666,6 @@ RSpec.describe UploadsController do ...@@ -666,6 +666,6 @@ RSpec.describe UploadsController do
def post_authorize(verified: true) def post_authorize(verified: true)
request.headers.merge!(workhorse_internal_api_request_header) if verified request.headers.merge!(workhorse_internal_api_request_header) if verified
post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json post :authorize, params: params, format: :json
end end
end end
...@@ -323,6 +323,16 @@ RSpec.shared_examples 'handle uploads authorize' do ...@@ -323,6 +323,16 @@ RSpec.shared_examples 'handle uploads authorize' do
end end
end end
context 'when id is not passed as a param' do
let(:params) { super().without(:id) }
it 'returns 404 status' do
post_authorize
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when a user can upload a file' do context 'when a user can upload a file' do
before do before do
sign_in(user) sign_in(user)
......
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