Merge branch '7-4-stable-sanitize' into '7-4-stable'

Sanitize fixes

See merge request !1264

3 files changed, 61 insertions, 3 deletions

diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index faa0ce67ca8..3e2c04f0969 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -1,4 +1,5 @@
 class Projects::BranchesController < Projects::ApplicationController
+  include ActionView::Helpers::SanitizeHelper
   # Authorize
   before_filter :authorize_read_project!
   before_filter :require_non_empty_project
@@ -17,8 +18,10 @@ class Projects::BranchesController < Projects::ApplicationController
   end
 
   def create
+    branch_name = sanitize(strip_tags(params[:branch_name]))
+    ref = sanitize(strip_tags(params[:ref]))
     result = CreateBranchService.new(project, current_user).
-        execute(params[:branch_name], params[:ref])
+        execute(branch_name, ref)
     if result[:status] == :success
       @branch = result[:branch]
       redirect_to project_tree_path(@project, @branch.name)
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 0e0532b65b2..36adeadd8a5 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -87,8 +87,8 @@ module CommitsHelper
   #  avatar: true will prepend the avatar image
   #  size:   size of the avatar image in px
   def commit_person_link(commit, options = {})
-    source_name = commit.send "#{options[:source]}_name".to_sym
-    source_email = commit.send "#{options[:source]}_email".to_sym
+    source_name = clean(commit.send "#{options[:source]}_name".to_sym)
+    source_email = clean(commit.send "#{options[:source]}_email".to_sym)
 
     user = User.find_for_commit(source_email, source_name)
     person_name = user.nil? ? source_name : user.name
@@ -124,4 +124,8 @@ module CommitsHelper
   def truncate_sha(sha)
     Commit.truncate_sha(sha)
   end
+
+  def clean(string)
+    Sanitize.clean(string, remove_contents: true)
+  end
 end
diff --git a/spec/controllers/branches_controller_spec.rb b/spec/controllers/branches_controller_spec.rb
new file mode 100644
index 00000000000..610d7a84e31
--- /dev/null
+++ b/spec/controllers/branches_controller_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe Projects::BranchesController do
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+
+  before do
+    sign_in(user)
+
+    project.team << [user, :master]
+
+    project.stub(:branches).and_return(['master', 'foo/bar/baz'])
+    project.stub(:tags).and_return(['v1.0.0', 'v2.0.0'])
+    controller.instance_variable_set(:@project, project)
+  end
+
+  describe "POST create" do
+    render_views
+
+    before {
+      post :create,
+        project_id: project.to_param,
+        branch_name: branch,
+        ref: ref
+    }
+
+    context "valid branch name, valid source" do
+      let(:branch) { "merge_branch" }
+      let(:ref) { "master" }
+      it { should redirect_to("/#{project.path_with_namespace}/tree/merge_branch") }
+    end
+
+    context "invalid branch name, valid ref" do
+      let(:branch) { "<script>alert('merge');</script>" }
+      let(:ref) { "master" }
+      it { should redirect_to("/#{project.path_with_namespace}/tree/alert('merge');") }
+    end
+
+    context "valid branch name, invalid ref" do
+      let(:branch) { "merge_branch" }
+      let(:ref) { "<script>alert('ref');</script>" }
+      it { should render_template("new") }
+    end
+
+    context "invalid branch name, invalid ref" do
+      let(:branch) { "<script>alert('merge');</script>" }
+      let(:ref) { "<script>alert('ref');</script>" }
+      it { should render_template("new") }
+    end
+  end
+end