diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-01-17 20:26:59 +0000 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-01-17 20:26:59 +0000 |
commit | f351cc28c2c878bf491bb0886be65bf35b58b261 (patch) | |
tree | 987d0a33d93dce35b4b25c401ae2c772760299d6 /spec/lib/gitlab | |
parent | 3b13159d9c83e8ce679663ce264854ea94bee8a2 (diff) | |
parent | d1eb3ff594b42d6e9625724119f52d3356045870 (diff) | |
download | gitlab-ce-f351cc28c2c878bf491bb0886be65bf35b58b261.tar.gz |
Merge branch 'sh-backport-10-3-4-security-fixes' into 'master'
Backport 10.3.4 security fixes into master
See merge request gitlab-org/gitlab-ce!16509
Diffstat (limited to 'spec/lib/gitlab')
-rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/ansi2html_spec.rb | 55 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/config/entry/key_spec.rb | 62 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/file_importer_spec.rb | 57 | ||||
-rw-r--r-- | spec/lib/gitlab/utils_spec.rb | 16 |
6 files changed, 181 insertions, 29 deletions
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index a6fbec295b5..cc202ce8bca 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -136,8 +136,8 @@ describe Gitlab::Auth do it 'grants deploy key write permissions' do project = create(:project) - key = create(:deploy_key, can_push: true) - create(:deploy_keys_project, deploy_key: key, project: project) + key = create(:deploy_key) + create(:deploy_keys_project, :write_access, deploy_key: key, project: project) token = Gitlab::LfsToken.new(key).token expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") @@ -146,7 +146,7 @@ describe Gitlab::Auth do it 'does not grant deploy key write permissions' do project = create(:project) - key = create(:deploy_key, can_push: true) + key = create(:deploy_key) token = Gitlab::LfsToken.new(key).token expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") diff --git a/spec/lib/gitlab/ci/ansi2html_spec.rb b/spec/lib/gitlab/ci/ansi2html_spec.rb index 05e2d94cbd6..7549e9941b6 100644 --- a/spec/lib/gitlab/ci/ansi2html_spec.rb +++ b/spec/lib/gitlab/ci/ansi2html_spec.rb @@ -217,11 +217,58 @@ describe Gitlab::Ci::Ansi2html do "#{section_end[0...-5]}</div>" end - it "prints light red" do - text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" - html = %{#{section_start_html}<span class="term-fg-l-red">Hello</span><br>#{section_end_html}} + shared_examples 'forbidden char in section_name' do + it 'ignores sections' do + text = "#{section_start}Some text#{section_end}" + html = text.gsub("\033[0K", '').gsub('<', '<') - expect(convert_html(text)).to eq(html) + expect(convert_html(text)).to eq(html) + end + end + + shared_examples 'a legit section' do + let(:text) { "#{section_start}Some text#{section_end}" } + + it 'prints light red' do + text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" + html = %{#{section_start_html}<span class="term-fg-l-red">Hello</span><br>#{section_end_html}} + + expect(convert_html(text)).to eq(html) + end + + it 'begins with a section_start html marker' do + expect(convert_html(text)).to start_with(section_start_html) + end + + it 'ends with a section_end html marker' do + expect(convert_html(text)).to end_with(section_end_html) + end + end + + it_behaves_like 'a legit section' + + context 'section name includes $' do + let(:section_name) { 'my_$ection'} + + it_behaves_like 'forbidden char in section_name' + end + + context 'section name includes <' do + let(:section_name) { '<a_tag>'} + + it_behaves_like 'forbidden char in section_name' + end + + context 'section name contains .-_' do + let(:section_name) { 'a.Legit-SeCtIoN_namE' } + + it_behaves_like 'a legit section' + end + + it 'do not allow XSS injections' do + text = "#{section_start}section_end:1:2<script>alert('XSS Hack!');</script>#{section_end}" + + expect(convert_html(text)).not_to include('<script>') end end diff --git a/spec/lib/gitlab/ci/config/entry/key_spec.rb b/spec/lib/gitlab/ci/config/entry/key_spec.rb index 5d4de60bc8a..3cbf19bea8b 100644 --- a/spec/lib/gitlab/ci/config/entry/key_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/key_spec.rb @@ -4,6 +4,26 @@ describe Gitlab::Ci::Config::Entry::Key do let(:entry) { described_class.new(config) } describe 'validations' do + shared_examples 'key with slash' do + it 'is invalid' do + expect(entry).not_to be_valid + end + + it 'reports errors with config value' do + expect(entry.errors).to include 'key config cannot contain the "/" character' + end + end + + shared_examples 'key with only dots' do + it 'is invalid' do + expect(entry).not_to be_valid + end + + it 'reports errors with config value' do + expect(entry.errors).to include 'key config cannot be "." or ".."' + end + end + context 'when entry config value is correct' do let(:config) { 'test' } @@ -30,6 +50,48 @@ describe Gitlab::Ci::Config::Entry::Key do end end end + + context 'when entry value contains slash' do + let(:config) { 'key/with/some/slashes' } + + it_behaves_like 'key with slash' + end + + context 'when entry value contains URI encoded slash (%2F)' do + let(:config) { 'key%2Fwith%2Fsome%2Fslashes' } + + it_behaves_like 'key with slash' + end + + context 'when entry value is a dot' do + let(:config) { '.' } + + it_behaves_like 'key with only dots' + end + + context 'when entry value is two dots' do + let(:config) { '..' } + + it_behaves_like 'key with only dots' + end + + context 'when entry value is a URI encoded dot (%2E)' do + let(:config) { '%2e' } + + it_behaves_like 'key with only dots' + end + + context 'when entry value is two URI encoded dots (%2E)' do + let(:config) { '%2E%2e' } + + it_behaves_like 'key with only dots' + end + + context 'when entry value is one dot and one URI encoded dot' do + let(:config) { '.%2e' } + + it_behaves_like 'key with only dots' + end end describe '.default' do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 4290fbb0087..2009a8ac48c 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -51,12 +51,12 @@ describe Gitlab::GitAccess do context 'when the project exists' do context 'when actor exists' do context 'when actor is a DeployKey' do - let(:deploy_key) { create(:deploy_key, user: user, can_push: true) } + let(:deploy_key) { create(:deploy_key, user: user) } let(:actor) { deploy_key } context 'when the DeployKey has access to the project' do before do - deploy_key.projects << project + deploy_key.deploy_keys_projects.create(project: project, can_push: true) end it 'allows push and pull access' do @@ -696,15 +696,13 @@ describe Gitlab::GitAccess do end describe 'deploy key permissions' do - let(:key) { create(:deploy_key, user: user, can_push: can_push) } + let(:key) { create(:deploy_key, user: user) } let(:actor) { key } context 'when deploy_key can push' do - let(:can_push) { true } - context 'when project is authorized' do before do - key.projects << project + key.deploy_keys_projects.create(project: project, can_push: true) end it { expect { push_access_check }.not_to raise_error } @@ -732,11 +730,9 @@ describe Gitlab::GitAccess do end context 'when deploy_key cannot push' do - let(:can_push) { false } - context 'when project is authorized' do before do - key.projects << project + key.deploy_keys_projects.create(project: project, can_push: false) end it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) } diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 162b776e107..5cdc5138fda 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -12,30 +12,61 @@ describe Gitlab::ImportExport::FileImporter do stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) - + allow(SecureRandom).to receive(:hex).and_return('abcd') setup_files - - described_class.import(archive_file: '', shared: shared) end after do FileUtils.rm_rf(export_path) end - it 'removes symlinks in root folder' do - expect(File.exist?(symlink_file)).to be false - end + context 'normal run' do + before do + described_class.import(archive_file: '', shared: shared) + end - it 'removes hidden symlinks in root folder' do - expect(File.exist?(hidden_symlink_file)).to be false - end + it 'removes symlinks in root folder' do + expect(File.exist?(symlink_file)).to be false + end + + it 'removes hidden symlinks in root folder' do + expect(File.exist?(hidden_symlink_file)).to be false + end + + it 'removes symlinks in subfolders' do + expect(File.exist?(subfolder_symlink_file)).to be false + end - it 'removes symlinks in subfolders' do - expect(File.exist?(subfolder_symlink_file)).to be false + it 'does not remove a valid file' do + expect(File.exist?(valid_file)).to be true + end + + it 'creates the file in the right subfolder' do + expect(shared.export_path).to include('test/abcd') + end end - it 'does not remove a valid file' do - expect(File.exist?(valid_file)).to be true + context 'error' do + before do + allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError) + described_class.import(archive_file: '', shared: shared) + end + + it 'removes symlinks in root folder' do + expect(File.exist?(symlink_file)).to be false + end + + it 'removes hidden symlinks in root folder' do + expect(File.exist?(hidden_symlink_file)).to be false + end + + it 'removes symlinks in subfolders' do + expect(File.exist?(subfolder_symlink_file)).to be false + end + + it 'does not remove a valid file' do + expect(File.exist?(valid_file)).to be true + end end def setup_files diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index e872a5290c5..bda239b7871 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -17,6 +17,22 @@ describe Gitlab::Utils do end end + describe '.remove_line_breaks' do + using RSpec::Parameterized::TableSyntax + + where(:original, :expected) do + "foo\nbar\nbaz" | "foobarbaz" + "foo\r\nbar\r\nbaz" | "foobarbaz" + "foobar" | "foobar" + end + + with_them do + it "replace line breaks with an empty string" do + expect(described_class.remove_line_breaks(original)).to eq(expected) + end + end + end + describe '.to_boolean' do it 'accepts booleans' do expect(to_boolean(true)).to be(true) |