Commit ef171dcf authored by Josianne Hyson's avatar Josianne Hyson

Exclude carrierwave remote url methods from import

Prior to this change, methods defined by carrierwave when using
`mount_uploader` could be used to supply remote urls to the project
importer. The method Note#remote_attachment_url could therefore be used
to perform SSRF attacks as this url was requested during the import.
The method `remote_attachment_request_header` could also be used to
supply data in these requests.

This commit filters these attributes out of the import as well as any
other dynamically generated attributes for models that have different
names for uploads.

This is implemented in carrierwave here:

https://github.com/carrierwaveuploader/carrierwave/blob/v1.3.1/lib/carrierwave/mount.rb
parent df4320ab
---
title: Exclude Carrierwave remote URL methods from import
merge_request:
author:
type: security
......@@ -11,7 +11,14 @@ module Gitlab
'discussion_id',
'custom_attributes'
].freeze
PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/, /attributes/).freeze
PROHIBITED_REFERENCES = Regexp.union(
/\Acached_markdown_version\Z/,
/_id\Z/,
/_ids\Z/,
/_html\Z/,
/attributes/,
/\Aremote_\w+_(url|urls|request_header)\Z/ # carrierwave automatically creates these attribute methods for uploads
).freeze
def self.clean(*args)
new(*args).clean
......
......@@ -32,6 +32,9 @@ describe Gitlab::ImportExport::AttributeCleaner do
'issue_ids' => [1, 2, 3],
'merge_request_ids' => [1, 2, 3],
'note_ids' => [1, 2, 3],
'remote_attachment_url' => 'http://something.dodgy',
'remote_attachment_request_header' => 'bad value',
'remote_attachment_urls' => %w(http://something.dodgy http://something.okay),
'attributes' => {
'issue_ids' => [1, 2, 3],
'merge_request_ids' => [1, 2, 3],
......
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