Commit 81b6c373 authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch '11-0-stable' into 11-0-stable-patch-2

parents 378bc0d1 bf968f8a
...@@ -2,6 +2,17 @@ ...@@ -2,6 +2,17 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
## 11.0.1 (2018-06-21)
### Security (5 changes)
- Fix XSS vulnerability for table of content generation.
- Update sanitize gem to 4.6.5 to fix HTML injection vulnerability.
- HTML escape branch name in project graphs page.
- HTML escape the name of the user in ProjectsHelper#link_to_member.
- Don't show events from internal projects for anonymous users in public feed.
## 11.0.0 (2018-06-22) ## 11.0.0 (2018-06-22)
### Security (3 changes) ### Security (3 changes)
......
...@@ -230,7 +230,7 @@ gem 'ruby-fogbugz', '~> 0.2.1' ...@@ -230,7 +230,7 @@ gem 'ruby-fogbugz', '~> 0.2.1'
gem 'kubeclient', '~> 3.1.0' gem 'kubeclient', '~> 3.1.0'
# Sanitize user input # Sanitize user input
gem 'sanitize', '~> 2.0' gem 'sanitize', '~> 4.6.5'
gem 'babosa', '~> 1.0.2' gem 'babosa', '~> 1.0.2'
# Sanitizes SVG input # Sanitizes SVG input
......
...@@ -296,13 +296,13 @@ GEM ...@@ -296,13 +296,13 @@ GEM
flowdock (~> 0.7) flowdock (~> 0.7)
gitlab-grit (>= 2.4.1) gitlab-grit (>= 2.4.1)
multi_json multi_json
gitlab-gollum-lib (4.2.7.2) gitlab-gollum-lib (4.2.7.5)
gemojione (~> 3.2) gemojione (~> 3.2)
github-markup (~> 1.6) github-markup (~> 1.6)
gollum-grit_adapter (~> 1.0) gollum-grit_adapter (~> 1.0)
nokogiri (>= 1.6.1, < 2.0) nokogiri (>= 1.6.1, < 2.0)
rouge (~> 3.1) rouge (~> 3.1)
sanitize (~> 2.1) sanitize (~> 4.6.4)
stringex (~> 2.6) stringex (~> 2.6)
gitlab-gollum-rugged_adapter (0.4.4) gitlab-gollum-rugged_adapter (0.4.4)
mime-types (>= 1.15) mime-types (>= 1.15)
...@@ -516,6 +516,8 @@ GEM ...@@ -516,6 +516,8 @@ GEM
netrc (0.11.0) netrc (0.11.0)
nokogiri (1.8.2) nokogiri (1.8.2)
mini_portile2 (~> 2.3.0) mini_portile2 (~> 2.3.0)
nokogumbo (1.5.0)
nokogiri
numerizer (0.1.1) numerizer (0.1.1)
oauth (0.5.4) oauth (0.5.4)
oauth2 (1.4.0) oauth2 (1.4.0)
...@@ -806,8 +808,10 @@ GEM ...@@ -806,8 +808,10 @@ GEM
et-orbi (~> 1.0) et-orbi (~> 1.0)
rugged (0.27.1) rugged (0.27.1)
safe_yaml (1.0.4) safe_yaml (1.0.4)
sanitize (2.1.0) sanitize (4.6.5)
crass (~> 1.0.2)
nokogiri (>= 1.4.4) nokogiri (>= 1.4.4)
nokogumbo (~> 1.4)
sass (3.5.5) sass (3.5.5)
sass-listen (~> 4.0.0) sass-listen (~> 4.0.0)
sass-listen (4.0.0) sass-listen (4.0.0)
...@@ -1154,7 +1158,7 @@ DEPENDENCIES ...@@ -1154,7 +1158,7 @@ DEPENDENCIES
ruby_parser (~> 3.8) ruby_parser (~> 3.8)
rufus-scheduler (~> 3.4) rufus-scheduler (~> 3.4)
rugged (~> 0.27) rugged (~> 0.27)
sanitize (~> 2.0) sanitize (~> 4.6.5)
sass-rails (~> 5.0.6) sass-rails (~> 5.0.6)
scss_lint (~> 0.56.0) scss_lint (~> 0.56.0)
seed-fu (~> 2.3.7) seed-fu (~> 2.3.7)
......
...@@ -56,7 +56,7 @@ class UserRecentEventsFinder ...@@ -56,7 +56,7 @@ class UserRecentEventsFinder
visible = target_user visible = target_user
.project_interactions .project_interactions
.where(visibility_level: [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]) .where(visibility_level: Gitlab::VisibilityLevel.levels_for_user(current_user))
.select(:id) .select(:id)
Gitlab::SQL::Union.new([authorized, visible]).to_sql Gitlab::SQL::Union.new([authorized, visible]).to_sql
......
...@@ -40,7 +40,8 @@ module ProjectsHelper ...@@ -40,7 +40,8 @@ module ProjectsHelper
name_tag_options[:class] << 'has-tooltip' name_tag_options[:class] << 'has-tooltip'
end end
content_tag(:span, sanitize(username), name_tag_options) # NOTE: ActionView::Helpers::TagHelper#content_tag HTML escapes username
content_tag(:span, username, name_tag_options)
end end
def link_to_member(project, author, opts = {}, &block) def link_to_member(project, author, opts = {}, &block)
......
...@@ -30,7 +30,7 @@ ...@@ -30,7 +30,7 @@
#{@commits_graph.start_date.strftime('%b %d')} #{@commits_graph.start_date.strftime('%b %d')}
- end_time = capture do - end_time = capture do
#{@commits_graph.end_date.strftime('%b %d')} #{@commits_graph.end_date.strftime('%b %d')}
= (_("Commit statistics for %{ref} %{start_time} - %{end_time}") % { ref: "<strong>#{@ref}</strong>", start_time: start_time, end_time: end_time }).html_safe = (_("Commit statistics for %{ref} %{start_time} - %{end_time}") % { ref: "<strong>#{h @ref}</strong>", start_time: start_time, end_time: end_time }).html_safe
.col-md-6 .col-md-6
.tree-ref-container .tree-ref-container
......
...@@ -25,10 +25,11 @@ module Banzai ...@@ -25,10 +25,11 @@ module Banzai
# Only push these customizations once # Only push these customizations once
return if customized?(whitelist[:transformers]) return if customized?(whitelist[:transformers])
# Allow table alignment; we whitelist specific style properties in a # Allow table alignment; we whitelist specific text-align values in a
# transformer below # transformer below
whitelist[:attributes]['th'] = %w(style) whitelist[:attributes]['th'] = %w(style)
whitelist[:attributes]['td'] = %w(style) whitelist[:attributes]['td'] = %w(style)
whitelist[:css] = { properties: ['text-align'] }
# Allow span elements # Allow span elements
whitelist[:elements].push('span') whitelist[:elements].push('span')
......
...@@ -92,7 +92,7 @@ module Banzai ...@@ -92,7 +92,7 @@ module Banzai
def text def text
return '' unless node return '' unless node
@text ||= node.text @text ||= EscapeUtils.escape_html(node.text)
end end
private private
......
...@@ -3,6 +3,7 @@ require 'spec_helper' ...@@ -3,6 +3,7 @@ require 'spec_helper'
describe 'Project Graph', :js do describe 'Project Graph', :js do
let(:user) { create :user } let(:user) { create :user }
let(:project) { create(:project, :repository, namespace: user.namespace) } let(:project) { create(:project, :repository, namespace: user.namespace) }
let(:branch_name) { 'master' }
before do before do
project.add_master(user) project.add_master(user)
...@@ -12,7 +13,7 @@ describe 'Project Graph', :js do ...@@ -12,7 +13,7 @@ describe 'Project Graph', :js do
shared_examples 'page should have commits graphs' do shared_examples 'page should have commits graphs' do
it 'renders commits' do it 'renders commits' do
expect(page).to have_content('Commit statistics for master') expect(page).to have_content("Commit statistics for #{branch_name}")
expect(page).to have_content('Commits per day of month') expect(page).to have_content('Commits per day of month')
end end
end end
...@@ -57,6 +58,23 @@ describe 'Project Graph', :js do ...@@ -57,6 +58,23 @@ describe 'Project Graph', :js do
it_behaves_like 'page should have languages graphs' it_behaves_like 'page should have languages graphs'
end end
context 'chart graph with HTML escaped branch name' do
let(:branch_name) { '<h1>evil</h1>' }
before do
project.repository.create_branch(branch_name, 'master')
visit charts_project_graph_path(project, branch_name)
end
it_behaves_like 'page should have commits graphs'
it 'HTML escapes branch name' do
expect(page.body).to include("Commit statistics for <strong>#{ERB::Util.html_escape(branch_name)}</strong>")
expect(page.body).not_to include(branch_name)
end
end
context 'when CI enabled' do context 'when CI enabled' do
before do before do
project.enable_ci project.enable_ci
......
require 'spec_helper' require 'spec_helper'
describe UserRecentEventsFinder do describe UserRecentEventsFinder do
let(:user) { create(:user) } let(:current_user) { create(:user) }
let(:project) { create(:project) } let(:project_owner) { create(:user) }
let(:project_owner) { project.creator } let(:private_project) { create(:project, :private, creator: project_owner) }
let!(:event) { create(:event, project: project, author: project_owner) } let(:internal_project) { create(:project, :internal, creator: project_owner) }
let(:public_project) { create(:project, :public, creator: project_owner) }
let!(:private_event) { create(:event, project: private_project, author: project_owner) }
let!(:internal_event) { create(:event, project: internal_project, author: project_owner) }
let!(:public_event) { create(:event, project: public_project, author: project_owner) }
subject(:finder) { described_class.new(user, project_owner) } subject(:finder) { described_class.new(current_user, project_owner) }
describe '#execute' do describe '#execute' do
it 'does not include the event when a user does not have access to the project' do context 'current user does not have access to projects' do
expect(finder.execute).to be_empty it 'returns public and internal events' do
records = finder.execute
expect(records).to include(public_event, internal_event)
expect(records).not_to include(private_event)
end
end end
context 'when the user has access to a project' do context 'when current user has access to the projects' do
before do before do
project.add_developer(user) private_project.add_developer(current_user)
internal_project.add_developer(current_user)
public_project.add_developer(current_user)
end end
it 'includes the event' do it 'returns all the events' do
expect(finder.execute).to include(event) expect(finder.execute).to include(private_event, internal_event, public_event)
end end
it 'does not include the event if the user cannot read cross project' do it 'does not include the events if the user cannot read cross project' do
expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } expect(Ability).to receive(:allowed?).with(current_user, :read_cross_project) { false }
expect(finder.execute).to be_empty expect(finder.execute).to be_empty
end end
end end
context 'when current user is anonymous' do
let(:current_user) { nil }
it 'returns public events only' do
expect(finder.execute).to eq([public_event])
end
end
end end
end end
...@@ -244,7 +244,7 @@ describe ProjectsHelper do ...@@ -244,7 +244,7 @@ describe ProjectsHelper do
describe '#link_to_member' do describe '#link_to_member' do
let(:group) { build_stubbed(:group) } let(:group) { build_stubbed(:group) }
let(:project) { build_stubbed(:project, group: group) } let(:project) { build_stubbed(:project, group: group) }
let(:user) { build_stubbed(:user) } let(:user) { build_stubbed(:user, name: '<h1>Administrator</h1>') }
describe 'using the default options' do describe 'using the default options' do
it 'returns an HTML link to the user' do it 'returns an HTML link to the user' do
...@@ -252,6 +252,13 @@ describe ProjectsHelper do ...@@ -252,6 +252,13 @@ describe ProjectsHelper do
expect(link).to match(%r{/#{user.username}}) expect(link).to match(%r{/#{user.username}})
end end
it 'HTML escapes the name of the user' do
link = helper.link_to_member(project, user)
expect(link).to include(ERB::Util.html_escape(user.name))
expect(link).not_to include(user.name)
end
end end
end end
......
...@@ -93,6 +93,16 @@ describe Banzai::Filter::SanitizationFilter do ...@@ -93,6 +93,16 @@ describe Banzai::Filter::SanitizationFilter do
expect(doc.at_css('td')['style']).to eq 'text-align: center' expect(doc.at_css('td')['style']).to eq 'text-align: center'
end end
it 'disallows `text-align` property in `style` attribute on other elements' do
html = <<~HTML
<div style="text-align: center">Text</div>
HTML
doc = filter(html)
expect(doc.at_css('div')['style']).to be_nil
end
it 'allows `span` elements' do it 'allows `span` elements' do
exp = act = %q{<span>Hello</span>} exp = act = %q{<span>Hello</span>}
expect(filter(act).to_html).to eq exp expect(filter(act).to_html).to eq exp
...@@ -224,7 +234,7 @@ describe Banzai::Filter::SanitizationFilter do ...@@ -224,7 +234,7 @@ describe Banzai::Filter::SanitizationFilter do
'protocol-based JS injection: spaces and entities' => { 'protocol-based JS injection: spaces and entities' => {
input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>', input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>',
output: '<a href="">foo</a>' output: '<a href>foo</a>'
}, },
'protocol whitespace' => { 'protocol whitespace' => {
......
...@@ -139,5 +139,14 @@ describe Banzai::Filter::TableOfContentsFilter do ...@@ -139,5 +139,14 @@ describe Banzai::Filter::TableOfContentsFilter do
expect(items[5].ancestors).to include(items[4]) expect(items[5].ancestors).to include(items[4])
end end
end end
context 'header text contains escaped content' do
let(:content) { '&lt;img src="x" onerror="alert(42)"&gt;' }
let(:results) { result(header(1, content)) }
it 'outputs escaped content' do
expect(doc.inner_html).to include(content)
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