Commit c3f62e0f authored by Laura Montemayor's avatar Laura Montemayor Committed by GitLab Release Tools Bot

Sanitize error input to prevent HTML/CSS injection in messages

Merge branch 'security-fix-html-css-injection-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2379

Changelog: security
parent 0a0775a3
...@@ -6,25 +6,28 @@ module Gitlab ...@@ -6,25 +6,28 @@ module Gitlab
module Chain module Chain
module Helpers module Helpers
def error(message, config_error: false, drop_reason: nil) def error(message, config_error: false, drop_reason: nil)
sanitized_message = ActionController::Base.helpers.sanitize(message, tags: [])
if config_error if config_error
drop_reason = :config_error drop_reason = :config_error
pipeline.yaml_errors = message pipeline.yaml_errors = sanitized_message
end end
pipeline.add_error_message(message) pipeline.add_error_message(sanitized_message)
drop_pipeline!(drop_reason) drop_pipeline!(drop_reason)
# TODO: consider not to rely on AR errors directly as they can be # TODO: consider not to rely on AR errors directly as they can be
# polluted with other unrelated errors (e.g. state machine) # polluted with other unrelated errors (e.g. state machine)
# https://gitlab.com/gitlab-org/gitlab/-/issues/220823 # https://gitlab.com/gitlab-org/gitlab/-/issues/220823
pipeline.errors.add(:base, message) pipeline.errors.add(:base, sanitized_message)
pipeline.errors.full_messages pipeline.errors.full_messages
end end
def warning(message) def warning(message)
pipeline.add_warning_message(message) sanitized_message = ActionController::Base.helpers.sanitize(message, tags: [])
pipeline.add_warning_message(sanitized_message)
end end
private private
......
...@@ -23,7 +23,7 @@ module Gitlab ...@@ -23,7 +23,7 @@ module Gitlab
end end
unless allowed_to_write_ref? unless allowed_to_write_ref?
error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance. <a href=https://docs.gitlab.com/ee/ci/pipelines/#pipeline-security-on-protected-branches>Learn more</a>".html_safe) error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance.")
end end
end end
......
...@@ -22,6 +22,19 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do ...@@ -22,6 +22,19 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do
let(:command) { double(save_incompleted: true) } let(:command) { double(save_incompleted: true) }
let(:message) { 'message' } let(:message) { 'message' }
describe '.warning' do
context 'when the warning includes malicious HTML' do
let(:message) { '<div>gimme your password</div>' }
let(:sanitized_message) { 'gimme your password' }
it 'sanitizes' do
subject.warning(message)
expect(pipeline.warning_messages[0].content).to include(sanitized_message)
end
end
end
describe '.error' do describe '.error' do
shared_examples 'error function' do shared_examples 'error function' do
specify do specify do
...@@ -36,6 +49,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do ...@@ -36,6 +49,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do
end end
end end
context 'when the error includes malicious HTML' do
let(:message) { '<div>gimme your password</div>' }
let(:sanitized_message) { 'gimme your password' }
it 'sanitizes the error and removes the HTML tags' do
subject.error(message, config_error: true, drop_reason: :config_error)
expect(pipeline.yaml_errors).to eq(sanitized_message)
expect(pipeline.errors[:base]).to include(sanitized_message)
end
end
context 'when given a drop reason' do context 'when given a drop reason' do
context 'when config error is true' do context 'when config error is true' do
context 'sets the yaml error and overrides the drop reason' do context 'sets the yaml error and overrides the drop reason' do
......
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