Add brakeman rake task and improve code security

5 files changed, 40 insertions, 15 deletions

diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb
index e2f957a640c..79d9910ce87 100644
--- a/app/controllers/projects/imports_controller.rb
+++ b/app/controllers/projects/imports_controller.rb
@@ -26,7 +26,7 @@ class Projects::ImportsController < Projects::ApplicationController
   def show
     unless @project.import_in_progress?
       if @project.import_finished?
-        redirect_to(@project) and return
+        redirect_to(project_path(@project)) and return
       else
         redirect_to new_namespace_project_import_path(@project.namespace,
                                                       @project) && return
diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb
index 71b0ab7ee82..f8a248ed729 100644
--- a/app/controllers/projects/team_members_controller.rb
+++ b/app/controllers/projects/team_members_controller.rb
@@ -15,15 +15,9 @@ class Projects::TeamMembersController < Projects::ApplicationController
 
   def create
     users = User.where(id: params[:user_ids].split(','))
-
     @project.team << [users, params[:access_level]]
 
-    if params[:redirect_to]
-      redirect_to params[:redirect_to]
-    else
-      redirect_to namespace_project_team_index_path(@project.namespace,
-                                                    @project)
-    end
+    redirect_to namespace_project_team_index_path(@project.namespace, @project)
   end
 
   def update
diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb
index 69824dca944..3392fbca91e 100644
--- a/app/controllers/projects/wikis_controller.rb
+++ b/app/controllers/projects/wikis_controller.rb
@@ -97,7 +97,7 @@ class Projects::WikisController < Projects::ApplicationController
     @project_wiki.wiki
   rescue ProjectWiki::CouldNotCreateWikiError => ex
     flash[:notice] = "Could not create Wiki Repository at this time. Please try again later."
-    redirect_to @project
+    redirect_to project_path(@project)
     return false
   end
 
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index b096c3913e1..810ac9f34bd 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -3,22 +3,53 @@ class UploadsController < ApplicationController
   before_filter :authorize_access
 
   def show
-    model = params[:model].camelize.constantize.find(params[:id])
-    uploader = model.send(params[:mounted_as])
+    unless upload_model && upload_mount
+      return not_found!
+    end
 
-    return not_found! if model.respond_to?(:project) && !can?(current_user, :read_project, model.project)
+    model = upload_model.find(params[:id])
+    uploader = model.send(upload_mount)
 
-    return redirect_to uploader.url unless uploader.file_storage?
+    if model.respond_to?(:project) && !can?(current_user, :read_project, model.project)
+      return not_found!
+    end
 
-    return not_found! unless uploader.file.exists?
+    unless uploader.file_storage?
+      return redirect_to uploader.url
+    end
+
+    unless uploader.file.exists?
+      return not_found!
+    end
 
     disposition = uploader.image? ? 'inline' : 'attachment'
     send_file uploader.file.path, disposition: disposition
   end
 
+  private
+
   def authorize_access
     unless params[:mounted_as] == 'avatar'
       authenticate_user! && reject_blocked!
     end
   end
+
+  def upload_model
+    upload_models = {
+      user: User,
+      project: Project,
+      note: Note,
+      group: Group
+    }
+
+    upload_models[params[:model].to_sym]
+  end
+
+  def upload_mount
+    upload_mounts = %w(avatar attachment file)
+
+    if upload_mounts.include?(params[:mounted_as])
+      params[:mounted_as]
+    end
+  end
 end
diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake
index 0a1e76ea822..abcb5f0ae46 100644
--- a/lib/tasks/brakeman.rake
+++ b/lib/tasks/brakeman.rake
@@ -1,6 +1,6 @@
 desc 'Security check via brakeman'
 task :brakeman do
-  if system("brakeman -w3 -z")
+  if system("brakeman --skip-files lib/backup/repository.rb -w3 -z")
     exit 0
   else
     puts 'Security check failed'