Commit 449e209e authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

parent ea1abf1e
......@@ -535,3 +535,27 @@ export function getURLOrigin(url) {
return null;
}
}
/**
* Returns `true` if the given `url` resolves to the same origin the page is served
* from; otherwise, returns `false`.
*
* The `url` may be absolute or relative.
*
* @param {string} url The URL to check.
* @returns {boolean}
*/
export function isSameOriginUrl(url) {
if (typeof url !== 'string') {
return false;
}
const { origin } = window.location;
try {
return new URL(url, origin).origin === origin;
} catch {
// Invalid URLs cannot have the same origin
return false;
}
}
......@@ -2,6 +2,7 @@
import { GlButton, GlFormInput, GlFormGroup, GlSprintf } from '@gitlab/ui';
import { mapState, mapActions, mapGetters } from 'vuex';
import { getParameterByName } from '~/lib/utils/common_utils';
import { isSameOriginUrl } from '~/lib/utils/url_utility';
import { __ } from '~/locale';
import MilestoneCombobox from '~/milestones/components/milestone_combobox.vue';
import { BACK_URL_PARAM } from '~/releases/constants';
......@@ -65,7 +66,13 @@ export default {
},
},
cancelPath() {
return getParameterByName(BACK_URL_PARAM) || this.releasesPagePath;
const backUrl = getParameterByName(BACK_URL_PARAM);
if (isSameOriginUrl(backUrl)) {
return backUrl;
}
return this.releasesPagePath;
},
saveButtonLabel() {
return this.isExistingRelease ? __('Save changes') : __('Create release');
......
......@@ -32,6 +32,9 @@ class AuditEvent < ApplicationRecord
scope :by_author_id, -> (author_id) { where(author_id: author_id) }
after_initialize :initialize_details
before_validation :sanitize_message
# Note: The intention is to remove this once refactoring of AuditEvent
# has proceeded further.
#
......@@ -83,6 +86,14 @@ class AuditEvent < ApplicationRecord
private
def sanitize_message
message = details[:custom_message]
return unless message
self.details = details.merge(custom_message: Sanitize.clean(message))
end
def default_author_value
::Gitlab::Audit::NullAuthor.for(author_id, (self[:author_name] || details[:author_name]))
end
......
......@@ -1255,12 +1255,23 @@ class User < ApplicationRecord
end
def sanitize_attrs
sanitize_links
sanitize_name
end
def sanitize_links
%i[skype linkedin twitter].each do |attr|
value = self[attr]
self[attr] = Sanitize.clean(value) if value.present?
end
end
def sanitize_name
return unless self.name
self.name = self.name.gsub(%r{</?[^>]*>}, '')
end
def set_notification_email
if notification_email.blank? || all_emails.exclude?(notification_email)
self.notification_email = email
......
......@@ -49,9 +49,9 @@ module FeatureFlags
end
def created_scope_message(scope)
"Created rule <strong>#{scope.environment_scope}</strong> "\
"and set it as <strong>#{scope.active ? "active" : "inactive"}</strong> "\
"with strategies <strong>#{scope.strategies}</strong>."
"Created rule #{scope.environment_scope} "\
"and set it as #{scope.active ? "active" : "inactive"} "\
"with strategies #{scope.strategies}."
end
def feature_flag_by_name
......
......@@ -22,8 +22,7 @@ module FeatureFlags
private
def audit_message(feature_flag)
message_parts = ["Created feature flag <strong>#{feature_flag.name}</strong>",
"with description <strong>\"#{feature_flag.description}\"</strong>."]
message_parts = ["Created feature flag #{feature_flag.name} with description \"#{feature_flag.description}\"."]
message_parts += feature_flag.scopes.map do |scope|
created_scope_message(scope)
......
......@@ -23,7 +23,7 @@ module FeatureFlags
end
def audit_message(feature_flag)
"Deleted feature flag <strong>#{feature_flag.name}</strong>."
"Deleted feature flag #{feature_flag.name}."
end
def can_destroy?(feature_flag)
......
......@@ -45,14 +45,14 @@ module FeatureFlags
return if changes.empty?
"Updated feature flag <strong>#{feature_flag.name}</strong>. " + changes.join(" ")
"Updated feature flag #{feature_flag.name}. " + changes.join(" ")
end
def changed_attributes_messages(feature_flag)
feature_flag.changes.slice(*AUDITABLE_ATTRIBUTES).map do |attribute_name, changes|
"Updated #{attribute_name} "\
"from <strong>\"#{changes.first}\"</strong> to "\
"<strong>\"#{changes.second}\"</strong>."
"from \"#{changes.first}\" to "\
"\"#{changes.second}\"."
end
end
......@@ -69,17 +69,17 @@ module FeatureFlags
end
def deleted_scope_message(scope)
"Deleted rule <strong>#{scope.environment_scope}</strong>."
"Deleted rule #{scope.environment_scope}."
end
def updated_scope_message(scope)
changes = scope.changes.slice(*AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES.keys)
return if changes.empty?
message = "Updated rule <strong>#{scope.environment_scope}</strong> "
message = "Updated rule #{scope.environment_scope} "
message += changes.map do |attribute_name, change|
name = AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES[attribute_name]
"#{name} from <strong>#{change.first}</strong> to <strong>#{change.second}</strong>"
"#{name} from #{change.first} to #{change.second}"
end.join(' ')
message + '.'
......
......@@ -65,18 +65,6 @@ RSpec.describe 'Comments on personal snippets', :js do
expect(page).to have_content(user_name)
end
end
context 'when the author name contains HTML' do
let(:user_name) { '<h1><a href="https://bad.link/malicious.exe" class="evil">Fake Content<img class="fake-icon" src="image.png"></a></h1>' }
it 'renders the name as plain text' do
visit snippet_path(snippet)
content = find("#note_#{snippet_notes[0].id} .note-header-author-name").text
expect(content).to eq user_name
end
end
end
context 'when submitting a note' do
......
import { TEST_HOST } from 'helpers/test_constants';
import * as urlUtils from '~/lib/utils/url_utility';
const shas = {
......@@ -921,4 +922,37 @@ describe('URL utility', () => {
expect(urlUtils.encodeSaferUrl(input)).toBe(input);
});
});
describe('isSameOriginUrl', () => {
// eslint-disable-next-line no-script-url
const javascriptUrl = 'javascript:alert(1)';
beforeEach(() => {
setWindowLocation({ origin: TEST_HOST });
});
it.each`
url | expected
${TEST_HOST} | ${true}
${`${TEST_HOST}/a/path`} | ${true}
${'//test.host/no-protocol'} | ${true}
${'/a/root/relative/path'} | ${true}
${'a/relative/path'} | ${true}
${'#hash'} | ${true}
${'?param=foo'} | ${true}
${''} | ${true}
${'../../../'} | ${true}
${`${TEST_HOST}:8080/wrong-port`} | ${false}
${'ws://test.host/wrong-protocol'} | ${false}
${'http://phishing.test'} | ${false}
${'//phishing.test'} | ${false}
${'//invalid:url'} | ${false}
${javascriptUrl} | ${false}
${'data:,Hello%2C%20World%21'} | ${false}
${null} | ${false}
${undefined} | ${false}
`('returns $expected given $url', ({ url, expected }) => {
expect(urlUtils.isSameOriginUrl(url)).toBe(expected);
});
});
});
......@@ -4,6 +4,7 @@ import MockAdapter from 'axios-mock-adapter';
import { merge } from 'lodash';
import Vuex from 'vuex';
import { getJSONFixture } from 'helpers/fixtures';
import { TEST_HOST } from 'helpers/test_constants';
import * as commonUtils from '~/lib/utils/common_utils';
import ReleaseEditNewApp from '~/releases/components/app_edit_new.vue';
import AssetLinksForm from '~/releases/components/asset_links_form.vue';
......@@ -11,6 +12,7 @@ import { BACK_URL_PARAM } from '~/releases/constants';
const originalRelease = getJSONFixture('api/releases/release.json');
const originalMilestones = originalRelease.milestones;
const releasesPagePath = 'path/to/releases/page';
describe('Release edit/new component', () => {
let wrapper;
......@@ -24,7 +26,7 @@ describe('Release edit/new component', () => {
state = {
release,
markdownDocsPath: 'path/to/markdown/docs',
releasesPagePath: 'path/to/releases/page',
releasesPagePath,
projectId: '8',
groupId: '42',
groupMilestonesAvailable: true,
......@@ -75,6 +77,8 @@ describe('Release edit/new component', () => {
};
beforeEach(() => {
global.jsdom.reconfigure({ url: TEST_HOST });
mock = new MockAdapter(axios);
gon.api_version = 'v4';
......@@ -146,22 +150,33 @@ describe('Release edit/new component', () => {
});
});
describe(`when the URL contains a "${BACK_URL_PARAM}" parameter`, () => {
const backUrl = 'https://example.gitlab.com/back/url';
// eslint-disable-next-line no-script-url
const xssBackUrl = 'javascript:alert(1)';
describe.each`
backUrl | expectedHref
${`${TEST_HOST}/back/url`} | ${`${TEST_HOST}/back/url`}
${`/back/url?page=2`} | ${`/back/url?page=2`}
${`back/url?page=3`} | ${`back/url?page=3`}
${'http://phishing.test/back/url'} | ${releasesPagePath}
${'//phishing.test/back/url'} | ${releasesPagePath}
${xssBackUrl} | ${releasesPagePath}
`(
`when the URL contains a "${BACK_URL_PARAM}=$backUrl" parameter`,
({ backUrl, expectedHref }) => {
beforeEach(async () => {
commonUtils.getParameterByName = jest
.fn()
.mockImplementation((paramToGet) => ({ [BACK_URL_PARAM]: backUrl }[paramToGet]));
global.jsdom.reconfigure({
url: `${TEST_HOST}?${BACK_URL_PARAM}=${encodeURIComponent(backUrl)}`,
});
await factory();
});
it('renders a "Cancel" button with an href pointing to the main Releases page', () => {
it(`renders a "Cancel" button with an href pointing to ${expectedHref}`, () => {
const cancelButton = wrapper.find('.js-cancel-button');
expect(cancelButton.attributes().href).toBe(backUrl);
});
expect(cancelButton.attributes().href).toBe(expectedHref);
});
},
);
describe('when creating a new release', () => {
beforeEach(async () => {
......
......@@ -3,9 +3,6 @@
require 'spec_helper'
RSpec.describe AuditEvent do
let_it_be(:audit_event) { create(:project_audit_event) }
subject { audit_event }
describe 'validations' do
include_examples 'validates IP address' do
let(:attribute) { :ip_address }
......@@ -13,6 +10,15 @@ RSpec.describe AuditEvent do
end
end
it 'sanitizes custom_message in the details hash' do
audit_event = create(:project_audit_event, details: { target_id: 678, custom_message: '<strong>Arnold</strong>' })
expect(audit_event.details).to include(
target_id: 678,
custom_message: 'Arnold'
)
end
describe '#as_json' do
context 'ip_address' do
subject { build(:group_audit_event, ip_address: '192.168.1.1').as_json }
......
......@@ -2877,7 +2877,7 @@ RSpec.describe User do
end
describe '#sanitize_attrs' do
let(:user) { build(:user, name: 'test & user', skype: 'test&user') }
let(:user) { build(:user, name: 'test <& user', skype: 'test&user') }
it 'encodes HTML entities in the Skype attribute' do
expect { user.sanitize_attrs }.to change { user.skype }.to('test&amp;user')
......@@ -2886,6 +2886,25 @@ RSpec.describe User do
it 'does not encode HTML entities in the name attribute' do
expect { user.sanitize_attrs }.not_to change { user.name }
end
it 'sanitizes attr from html tags' do
user = create(:user, name: '<a href="//example.com">Test<a>', twitter: '<a href="//evil.com">https://twitter.com<a>')
expect(user.name).to eq('Test')
expect(user.twitter).to eq('https://twitter.com')
end
it 'sanitizes attr from js scripts' do
user = create(:user, name: '<script>alert("Test")</script>')
expect(user.name).to eq("alert(\"Test\")")
end
it 'sanitizes attr from iframe scripts' do
user = create(:user, name: 'User"><iframe src=javascript:alert()><iframe>')
expect(user.name).to eq('User">')
end
end
describe '#starred?' do
......
......@@ -68,12 +68,12 @@ RSpec.describe FeatureFlags::CreateService do
end
it 'creates audit event' do
expected_message = 'Created feature flag <strong>feature_flag</strong> '\
'with description <strong>"description"</strong>. '\
'Created rule <strong>*</strong> and set it as <strong>active</strong> '\
'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>. '\
'Created rule <strong>production</strong> and set it as <strong>inactive</strong> '\
'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>.'
expected_message = 'Created feature flag feature_flag '\
'with description "description". '\
'Created rule * and set it as active '\
'with strategies [{"name"=&gt;"default", "parameters"=&gt;{}}]. '\
'Created rule production and set it as inactive '\
'with strategies [{"name"=&gt;"default", "parameters"=&gt;{}}].'
expect { subject }.to change { AuditEvent.count }.by(1)
expect(AuditEvent.last.details[:custom_message]).to eq(expected_message)
......
......@@ -33,7 +33,7 @@ RSpec.describe FeatureFlags::DestroyService do
it 'creates audit log' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to eq("Deleted feature flag <strong>#{feature_flag.name}</strong>.")
expect(audit_event_message).to eq("Deleted feature flag #{feature_flag.name}.")
end
context 'when user is reporter' do
......
......@@ -38,9 +38,9 @@ RSpec.describe FeatureFlags::UpdateService do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
eq("Updated feature flag <strong>new_name</strong>. "\
"Updated name from <strong>\"#{name_was}\"</strong> "\
"to <strong>\"new_name\"</strong>.")
eq("Updated feature flag new_name. "\
"Updated name from \"#{name_was}\" "\
"to \"new_name\".")
)
end
......@@ -94,8 +94,8 @@ RSpec.describe FeatureFlags::UpdateService do
it 'creates audit event with changed description' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include("Updated description from <strong>\"\"</strong>"\
" to <strong>\"new description\"</strong>.")
include("Updated description from \"\""\
" to \"new description\".")
)
end
end
......@@ -110,7 +110,7 @@ RSpec.describe FeatureFlags::UpdateService do
it 'creates audit event about changing active state' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include('Updated active from <strong>"true"</strong> to <strong>"false"</strong>.')
include('Updated active from "true" to "false".')
)
end
......@@ -132,8 +132,8 @@ RSpec.describe FeatureFlags::UpdateService do
it 'creates audit event about changing active state' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include("Updated rule <strong>*</strong> active state "\
"from <strong>true</strong> to <strong>false</strong>.")
include("Updated rule * active state "\
"from true to false.")
)
end
end
......@@ -149,8 +149,8 @@ RSpec.describe FeatureFlags::UpdateService do
it 'creates audit event with changed name' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include("Updated rule <strong>staging</strong> environment scope "\
"from <strong>review</strong> to <strong>staging</strong>.")
include("Updated rule staging environment scope "\
"from review to staging.")
)
end
......@@ -185,7 +185,7 @@ RSpec.describe FeatureFlags::UpdateService do
it 'creates audit event with deleted scope' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to include("Deleted rule <strong>review</strong>.")
expect(audit_event_message).to include("Deleted rule review.")
end
context 'when scope can not be deleted' do
......@@ -210,8 +210,8 @@ RSpec.describe FeatureFlags::UpdateService do
end
it 'creates audit event with new scope' do
expected = 'Created rule <strong>review</strong> and set it as <strong>active</strong> '\
'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>.'
expected = 'Created rule review and set it as active '\
'with strategies [{"name"=&gt;"default", "parameters"=&gt;{}}].'
subject
......@@ -260,7 +260,7 @@ RSpec.describe FeatureFlags::UpdateService do
end
it 'creates an audit event' do
expected = %r{Updated rule <strong>sandbox</strong> strategies from <strong>.*</strong> to <strong>.*</strong>.}
expected = %r{Updated rule sandbox strategies from .* to .*.}
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to match(expected)
......
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