Commit bf7932bd authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'dir-traversal' into 'master'

Fix directory traversal vulnerabilities

Fixes gitlab/gitlab-ee#272.

As @joern mentions:

> This is not exploitable via the front-end nginx. But nevertheless this issue should be addressed.

See merge request !1760
parents 582bff2c 5e2f25c3
...@@ -4,6 +4,8 @@ v 7.10.0 (unreleased) ...@@ -4,6 +4,8 @@ v 7.10.0 (unreleased)
- Fix broken file browsing with a submodule that contains a relative link (Stan Hu) - Fix broken file browsing with a submodule that contains a relative link (Stan Hu)
- Fix persistent XSS vulnerability around profile website URLs. - Fix persistent XSS vulnerability around profile website URLs.
- Fix project import URL regex to prevent arbitary local repos from being imported. - Fix project import URL regex to prevent arbitary local repos from being imported.
- Fix directory traversal vulnerability around uploads routes.
- Fix directory traversal vulnerability around help pages.
- Fix bug where Wiki pages that included a '/' were no longer accessible (Stan Hu) - Fix bug where Wiki pages that included a '/' were no longer accessible (Stan Hu)
- Fix bug where error messages from Dropzone would not be displayed on the issues page (Stan Hu) - Fix bug where error messages from Dropzone would not be displayed on the issues page (Stan Hu)
- Add ability to configure Reply-To address in gitlab.yml (Stan Hu) - Add ability to configure Reply-To address in gitlab.yml (Stan Hu)
......
...@@ -3,7 +3,7 @@ class HelpController < ApplicationController ...@@ -3,7 +3,7 @@ class HelpController < ApplicationController
end end
def show def show
@filepath = params[:filepath] @filepath = clean_path_info(params[:filepath])
@format = params[:format] @format = params[:format]
respond_to do |format| respond_to do |format|
...@@ -36,4 +36,35 @@ class HelpController < ApplicationController ...@@ -36,4 +36,35 @@ class HelpController < ApplicationController
def ui def ui
end end
PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact)
# Taken from ActionDispatch::FileHandler
# Cleans up the path, to prevent directory traversal outside the doc folder.
def clean_path_info(path_info)
parts = path_info.split(PATH_SEPS)
clean = []
# Walk over each part of the path
parts.each do |part|
# Turn `one//two` or `one/./two` into `one/two`.
next if part.empty? || part == '.'
if part == '..'
# Turn `one/two/../` into `one`
clean.pop
else
# Add simple folder names to the clean path.
clean << part
end
end
# If the path was an absolute path (i.e. `/` or `/one/two`),
# add `/` to the front of the clean path.
clean.unshift '/' if parts.empty? || parts.first.empty?
# Join all the clean path parts by the path separator.
::File.join(*clean)
end
end end
...@@ -91,18 +91,18 @@ Gitlab::Application.routes.draw do ...@@ -91,18 +91,18 @@ Gitlab::Application.routes.draw do
# Note attachments and User/Group/Project avatars # Note attachments and User/Group/Project avatars
get ":model/:mounted_as/:id/:filename", get ":model/:mounted_as/:id/:filename",
to: "uploads#show", to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /.+/ } constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
# Project markdown uploads # Project markdown uploads
get ":namespace_id/:project_id/:secret/:filename", get ":namespace_id/:project_id/:secret/:filename",
to: "projects/uploads#show", to: "projects/uploads#show",
constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /.+/ } constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /[^\/]+/ }
end end
# Redirect old note attachments path to new uploads path. # Redirect old note attachments path to new uploads path.
get "files/note/:id/:filename", get "files/note/:id/:filename",
to: redirect("uploads/note/attachment/%{id}/%{filename}"), to: redirect("uploads/note/attachment/%{id}/%{filename}"),
constraints: { filename: /.+/ } constraints: { filename: /[^\/]+/ }
# #
# Explore area # Explore area
...@@ -487,7 +487,7 @@ Gitlab::Application.routes.draw do ...@@ -487,7 +487,7 @@ Gitlab::Application.routes.draw do
resources :uploads, only: [:create] do resources :uploads, only: [:create] do
collection do collection do
get ":secret/:filename", action: :show, as: :show, constraints: { filename: /.+/ } get ":secret/:filename", action: :show, as: :show, constraints: { filename: /[^\/]+/ }
end end
end end
end end
......
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