Commit a5faa56b authored by Cindy Pallares's avatar Cindy Pallares

Merge branch 'security-stored-xss-for-environments' into 'master'

[master] Stored XSS for Environments

Closes #2727

See merge request gitlab/gitlabhq!2594
parents 81d55bb0 2f89c77e
---
title: Fix stored XSS for Environments
merge_request:
author:
type: security
# frozen_string_literal: true
class CleanupEnvironmentsExternalUrl < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
update_column_in_batches(:environments, :external_url, nil) do |table, query|
query.where(table[:external_url].matches('javascript://%'))
end
end
def down
end
end
......@@ -111,12 +111,14 @@ module Gitlab
end
def internal_web?(uri)
uri.hostname == config.gitlab.host &&
uri.scheme == config.gitlab.protocol &&
uri.hostname == config.gitlab.host &&
(uri.port.blank? || uri.port == config.gitlab.port)
end
def internal_shell?(uri)
uri.hostname == config.gitlab_shell.ssh_host &&
uri.scheme == 'ssh' &&
uri.hostname == config.gitlab_shell.ssh_host &&
(uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
end
......
......@@ -308,7 +308,7 @@ FactoryBot.define do
trait :with_runner_session do
after(:build) do |build|
build.build_runner_session(url: 'ws://localhost')
build.build_runner_session(url: 'https://localhost')
end
end
end
......
......@@ -10,8 +10,8 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?(import_url)).to be false
end
it 'allows imports from configured SSH host and port' do
import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
it 'allows mirroring from configured SSH host and port' do
import_url = "ssh://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
expect(described_class.blocked_url?(import_url)).to be false
end
......@@ -29,6 +29,14 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true
end
it 'returns true for bad protocol on configured web/SSH host and ports' do
web_url = "javascript://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git%0aalert(1)"
expect(described_class.blocked_url?(web_url)).to be true
ssh_url = "javascript://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git%0aalert(1)"
expect(described_class.blocked_url?(ssh_url)).to be true
end
it 'returns true for localhost IPs' do
expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true
expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true
......
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20181108091549_cleanup_environments_external_url.rb')
describe CleanupEnvironmentsExternalUrl, :migration do
let(:environments) { table(:environments) }
let(:invalid_entries) { environments.where(environments.arel_table[:external_url].matches('javascript://%')) }
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
before do
namespace = namespaces.create(name: 'foo', path: 'foo')
project = projects.create!(namespace_id: namespace.id)
environments.create!(id: 1, project_id: project.id, name: 'poisoned', slug: 'poisoned', external_url: 'javascript://alert("1")')
end
it 'clears every environment with a javascript external_url' do
expect do
subject.up
end.to change { invalid_entries.count }.from(1).to(0)
end
it 'do not removes environments' do
expect do
subject.up
end.not_to change { environments.count }
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