diff options
Diffstat (limited to 'spec/lib/gitlab')
| -rw-r--r-- | spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb | 41 | ||||
| -rw-r--r-- | spec/lib/gitlab/middleware/multipart_with_handler_spec.rb | 52 | ||||
| -rw-r--r-- | spec/lib/gitlab/regex_spec.rb | 13 | ||||
| -rw-r--r-- | spec/lib/gitlab/utils_spec.rb | 31 |
4 files changed, 128 insertions, 9 deletions
diff --git a/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb b/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb index 875e3820011..a1e9ac6e425 100644 --- a/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb @@ -123,15 +123,46 @@ RSpec.describe Gitlab::Middleware::Multipart do end end - context 'with invalid key in parameters' do + context 'with an invalid upload key' do include_context 'with one temporary file for multipart' - let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } - let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'wrong_key', filename: filename, remote_id: remote_id) } + RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:| + let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) } - it 'raises an error' do - expect { subject }.to raise_error(RuntimeError, 'Empty JWT param: file.gitlab-workhorse-upload') + it 'raises an error' do + expect { subject }.to raise_error(RuntimeError, error_message) + end end + + it_behaves_like 'rejecting the invalid key', + key_in_header: 'file', + key_in_upload_params: 'wrong_key', + error_message: 'Empty JWT param: file.gitlab-workhorse-upload' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[user]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[user]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar[image[url]]]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar[image[url]]]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'x' * 11000, + key_in_upload_params: 'user[avatar]', + error_message: "invalid field: \"#{'x' * 11000}\"" end context 'with a modified JWT payload' do diff --git a/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb b/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb index 742a5639ace..8c2af775574 100644 --- a/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb @@ -139,6 +139,58 @@ RSpec.describe Gitlab::Middleware::Multipart do subject end end + + context 'with invalid key in header' do + include_context 'with one temporary file for multipart' + + RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:| + let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) } + + it 'raises an error' do + expect { subject }.to raise_error(RuntimeError, error_message) + end + end + + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[user]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[user]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar[image[url]]]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar[image[url]]]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'x' * 11000, + key_in_upload_params: 'user[avatar]', + error_message: "invalid field: \"#{'x' * 11000}\"" + end + + context 'with key with unbalanced brackets in header' do + include_context 'with one temporary file for multipart' + + let(:invalid_key) { 'user[avatar' } + let(:rewritten_fields) { rewritten_fields_hash( invalid_key => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'user[avatar]', filename: filename, remote_id: remote_id) } + + it 'builds no UploadedFile' do + expect(app).not_to receive(:call) + + expect { subject }.to raise_error(RuntimeError, "invalid field: \"#{invalid_key}\"") + end + end end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 1c56e489a94..66ed80a7d61 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -137,11 +137,16 @@ RSpec.describe Gitlab::Regex do it { is_expected.to match('my/awesome/image-1') } it { is_expected.to match('my/awesome/image.test') } it { is_expected.to match('my/awesome/image--test') } - # docker distribution allows for infinite `-` - # https://github.com/docker/distribution/blob/master/reference/regexp.go#L13 - # but we have a range of 0,10 to add a reasonable limit. - it { is_expected.not_to match('my/image-----------test') } + it { is_expected.to match('my/image__test') } + # this example tests for catastrophic backtracking + it { is_expected.to match('user1/project/a_bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb------------x') } + it { is_expected.not_to match('user1/project/a_bbbbb-------------') } it { is_expected.not_to match('my/image-.test') } + it { is_expected.not_to match('my/image___test') } + it { is_expected.not_to match('my/image_.test') } + it { is_expected.not_to match('my/image_-test') } + it { is_expected.not_to match('my/image..test') } + it { is_expected.not_to match('my/image\ntest') } it { is_expected.not_to match('.my/image') } it { is_expected.not_to match('my/image.') } end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 1eaceec1d8a..36257a0605b 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Utils do + using RSpec::Parameterized::TableSyntax + delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes, :append_path, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, :decode_path, :ms_to_round_sec, to: :described_class @@ -50,6 +52,10 @@ RSpec.describe Gitlab::Utils do expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb') expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') end + + it 'does nothing for a non-string' do + expect(check_path_traversal!(nil)).to be_nil + end end describe '.allowlisted?' do @@ -448,4 +454,29 @@ RSpec.describe Gitlab::Utils do end end end + + describe '.valid_brackets?' do + where(:input, :allow_nested, :valid) do + 'no brackets' | true | true + 'no brackets' | false | true + 'user[avatar]' | true | true + 'user[avatar]' | false | true + 'user[avatar][friends]' | true | true + 'user[avatar][friends]' | false | true + 'user[avatar[image[url]]]' | true | true + 'user[avatar[image[url]]]' | false | false + 'user[avatar[]friends]' | true | true + 'user[avatar[]friends]' | false | false + 'user[avatar]]' | true | false + 'user[avatar]]' | false | false + 'user][avatar]]' | true | false + 'user][avatar]]' | false | false + 'user[avatar' | true | false + 'user[avatar' | false | false + end + + with_them do + it { expect(described_class.valid_brackets?(input, allow_nested: allow_nested)).to eq(valid) } + end + end end |