Add FileUploader.root to allowed upload paths

Currently we check if uploaded file is under
`Gitlab.config.uploads.storage_path`, the problem is that
uploads are placed in `uploads` subdirectory which is symlink.

In allow_path? method we check real (expanded) paths, which causes
that `Gitlab.config.uploads.storage_path` is expaned into symlink
path and there is a mismatch with upload file path.

By adding `Gitlab.config.uploads.storage_path/uploads` into allowed
paths, this path is expaned during path check.

`Gitlab.config.uploads.storage_path` is left there intentionally in case
some uploader wouldn't use `uploads` subdir.
This commit is contained in:
Jan Provaznik 2018-07-07 19:30:16 +02:00
parent 96eb6fd33b
commit e2ec97a92e
3 changed files with 9 additions and 3 deletions

View file

@ -0,0 +1,5 @@
---
title: Add /uploads subdirectory to allowed upload paths.
merge_request:
author:
type: fixed

View file

@ -84,7 +84,7 @@ module Gitlab
def open_file(params, key)
::UploadedFile.from_params(
params, key,
Gitlab.config.uploads.storage_path)
[FileUploader.root, Gitlab.config.uploads.storage_path])
end
end

View file

@ -28,7 +28,7 @@ class UploadedFile
@tempfile = File.new(path, 'rb')
end
def self.from_params(params, field, upload_path)
def self.from_params(params, field, upload_paths)
unless params["#{field}.path"]
raise InvalidPathError, "file is invalid" if params["#{field}.remote_id"]
@ -37,7 +37,8 @@ class UploadedFile
file_path = File.realpath(params["#{field}.path"])
unless self.allowed_path?(file_path, [upload_path, Dir.tmpdir].compact)
paths = Array.wrap(upload_paths) << Dir.tmpdir
unless self.allowed_path?(file_path, paths.compact)
raise InvalidPathError, "insecure path used '#{file_path}'"
end