Merge branch 'brakeman' into 'master'

Add Brakeman - Static analysis security scanner for Ruby on Rails

See merge request !1616

10 files changed, 75 insertions, 16 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 84bdea30979..6a28772097e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@ v 7.9.0 (unreleased)
   - Fix mass-unassignment of issues (Robert Speicher)
   - Allow user confirmation to be skipped for new users via API
   - Add a service to send updates to an Irker gateway (Romain Coltel)
+  - Add brakeman (security scanner for Ruby on Rails)
 
 v 7.8.1
   - Fix run of custom post receive hooks
diff --git a/Gemfile b/Gemfile
index 47e6fe95f34..01c02b5c8db 100644
--- a/Gemfile
+++ b/Gemfile
@@ -199,6 +199,7 @@ gem "virtus"
 gem 'addressable'
 
 group :development do
+  gem 'brakeman', require: false
   gem "annotate", "~> 2.6.0.beta2"
   gem "letter_opener"
   gem 'quiet_assets', '~> 1.0.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index 37880c45a29..102d1a28875 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -63,6 +63,16 @@ GEM
     bootstrap-sass (3.3.3)
       autoprefixer-rails (>= 5.0.0.1)
       sass (>= 3.2.19)
+    brakeman (3.0.1)
+      erubis (~> 2.6)
+      fastercsv (~> 1.5)
+      haml (>= 3.0, < 5.0)
+      highline (~> 1.6.20)
+      multi_json (~> 1.2)
+      ruby2ruby (~> 2.1.1)
+      ruby_parser (~> 3.5.0)
+      sass (~> 3.0)
+      terminal-table (~> 1.4)
     browser (0.7.2)
     builder (3.2.2)
     byebug (3.2.0)
@@ -154,6 +164,7 @@ GEM
       multipart-post (~> 1.2.0)
     faraday_middleware (0.9.0)
       faraday (>= 0.7.4, < 0.9)
+    fastercsv (1.5.5)
     ffaker (1.22.1)
     ffi (1.9.3)
     fog (1.21.0)
@@ -258,6 +269,7 @@ GEM
       haml (>= 3.1, < 5.0)
       railties (>= 4.0.1)
     hashie (2.1.2)
+    highline (1.6.21)
     hike (1.2.3)
     hipchat (1.4.0)
       httparty
@@ -496,6 +508,11 @@ GEM
       rainbow (>= 1.99.1, < 3.0)
       ruby-progressbar (~> 1.4)
     ruby-progressbar (1.7.1)
+    ruby2ruby (2.1.3)
+      ruby_parser (~> 3.1)
+      sexp_processor (~> 4.0)
+    ruby_parser (3.5.0)
+      sexp_processor (~> 4.1)
     rubyntlm (0.4.0)
     rubypants (0.2.0)
     rugged (0.21.4)
@@ -521,6 +538,7 @@ GEM
     select2-rails (3.5.2)
       thor (~> 0.14)
     settingslogic (2.0.9)
+    sexp_processor (4.4.5)
     shoulda-matchers (2.7.0)
       activesupport (>= 3.0.0)
     sidekiq (3.3.0)
@@ -572,6 +590,7 @@ GEM
     temple (0.6.7)
     term-ansicolor (1.2.2)
       tins (~> 0.8)
+    terminal-table (1.4.5)
     test_after_commit (0.2.2)
     therubyracer (0.12.0)
       libv8 (~> 3.16.14.0)
@@ -651,6 +670,7 @@ DEPENDENCIES
   better_errors
   binding_of_caller
   bootstrap-sass (~> 3.0)
+  brakeman
   browser
   byebug
   cal-heatmap-rails (~> 0.0.1)
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/doc/development/ci_setup.md b/doc/development/ci_setup.md
index f417667754e..f9b48868182 100644
--- a/doc/development/ci_setup.md
+++ b/doc/development/ci_setup.md
@@ -37,7 +37,10 @@ bundle install --deployment --path vendor/bundle (Setup)
 cp config/gitlab.yml.example config/gitlab.yml (Setup)
 bundle exec rake db:create (Setup)
 bundle exec rake spinach (Thread #1)
-bundle exec rake spec (Thread #2)
+bundle exec rake spec (thread #2)
+bundle exec rake rubocop (thread #3)
+bundle exec rake brakeman (thread #4)
+bundle exec rake jasmine:ci (thread #5)
 ```
 
 Use rubygems mirror.
diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake
new file mode 100644
index 00000000000..abcb5f0ae46
--- /dev/null
+++ b/lib/tasks/brakeman.rake
@@ -0,0 +1,9 @@
+desc 'Security check via brakeman'
+task :brakeman do
+  if system("brakeman --skip-files lib/backup/repository.rb -w3 -z")
+    exit 0
+  else
+    puts 'Security check failed'
+    exit 1
+  end
+end
diff --git a/lib/tasks/test.rake b/lib/tasks/test.rake
index 3ea9290a814..a39d9649876 100644
--- a/lib/tasks/test.rake
+++ b/lib/tasks/test.rake
@@ -9,5 +9,5 @@ unless Rails.env.production?
   require 'coveralls/rake/task'
   Coveralls::RakeTask.new
   desc "GITLAB | Run all tests on CI with simplecov"
-  task :test_ci => [:rubocop, :spinach, :spec, 'coveralls:push']
+  task :test_ci => [:rubocop, :brakeman, 'jasmine:ci', :spinach, :spec, 'coveralls:push']
 end