diff options
author | Thiago Presa <tpresa@gitlab.com> | 2018-10-24 21:07:44 +0000 |
---|---|---|
committer | Thiago Presa <tpresa@gitlab.com> | 2018-10-24 21:40:03 -0300 |
commit | fd080530e5de0e890eccfc917d4ba40c0b40c564 (patch) | |
tree | 6e2bd81a5e1e8ac49901f551d7eb286b41e511fc | |
parent | bb3ad8e7a19ad1e2b70537f44453e0030fe741d5 (diff) | |
download | gitlab-ce-fd080530e5de0e890eccfc917d4ba40c0b40c564.tar.gz |
Merge branch 'sh-validate-wiki-attachments-11-3' into 'security-11-3'
[11.3] Validate Wiki attachments are valid temporary files
See merge request gitlab/gitlabhq!2570
-rw-r--r-- | changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml | 5 | ||||
-rw-r--r-- | lib/api/validations/types/safe_file.rb | 15 | ||||
-rw-r--r-- | lib/api/wikis.rb | 4 | ||||
-rw-r--r-- | spec/requests/api/wikis_spec.rb | 10 |
4 files changed, 32 insertions, 2 deletions
diff --git a/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml b/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml new file mode 100644 index 00000000000..ac6ab7cc3f4 --- /dev/null +++ b/changelogs/unreleased/sh-fix-wiki-security-issue-53072.yml @@ -0,0 +1,5 @@ +--- +title: Validate Wiki attachments are valid temporary files +merge_request: +author: +type: security diff --git a/lib/api/validations/types/safe_file.rb b/lib/api/validations/types/safe_file.rb new file mode 100644 index 00000000000..53b5790bfa2 --- /dev/null +++ b/lib/api/validations/types/safe_file.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# This module overrides the Grape type validator defined in +# https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/types/file.rb +module API + module Validations + module Types + class SafeFile < ::Grape::Validations::Types::File + def value_coerced?(value) + super && value[:tempfile].is_a?(Tempfile) + end + end + end + end +end diff --git a/lib/api/wikis.rb b/lib/api/wikis.rb index e86ebc573f2..c98f6dd9318 100644 --- a/lib/api/wikis.rb +++ b/lib/api/wikis.rb @@ -4,7 +4,7 @@ module API def commit_params(attrs) { file_name: attrs[:file][:filename], - file_content: File.read(attrs[:file][:tempfile]), + file_content: attrs[:file][:tempfile].read, branch_name: attrs[:branch] } end @@ -98,7 +98,7 @@ module API success Entities::WikiAttachment end params do - requires :file, type: File, desc: 'The attachment file to be uploaded' + requires :file, type: ::API::Validations::Types::SafeFile, desc: 'The attachment file to be uploaded' optional :branch, type: String, desc: 'The name of the branch' end post ":id/wikis/attachments", requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index c40d01e1a14..08bada44178 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -158,6 +158,16 @@ describe API::Wikis do expect(json_response.size).to eq(1) expect(json_response['error']).to eq('file is missing') end + + it 'responds with validation error on invalid temp file' do + payload[:file] = { tempfile: '/etc/hosts' } + + post(api(url, user), payload) + + expect(response).to have_gitlab_http_status(400) + expect(json_response.size).to eq(1) + expect(json_response['error']).to eq('file is invalid') + end end describe 'GET /projects/:id/wikis' do |