Commit 5dc00c45 authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Stan Hu

Reject incomplete multibyte strings

Trying to encode the input to UTF8 before matching would cause errors
when the string includes incomplete multibyte characters or any
characters ruby can't convert to UTF8.
parent a9bb1ef7
---
title: Reject incomplete multibyte chars in UTF8 params
merge_request: 47658
author:
type: fixed
...@@ -89,8 +89,12 @@ module Gitlab ...@@ -89,8 +89,12 @@ module Gitlab
def string_malformed?(string) def string_malformed?(string)
# We're using match rather than include, because that will raise an ArgumentError # We're using match rather than include, because that will raise an ArgumentError
# when the string contains invalid UTF8 # when the string contains invalid UTF8
string.match?(NULL_BYTE_REGEX) #
rescue ArgumentError # We try to encode the string from ASCII-8BIT to UTF8. If we failed to do
# so for certain characters in the string, those chars are probably incomplete
# multibyte characters.
string.encode(Encoding::UTF_8).match?(NULL_BYTE_REGEX)
rescue ArgumentError, Encoding::UndefinedConversionError
# If we're here, we caught a malformed string. Return true # If we're here, we caught a malformed string. Return true
true true
end end
......
...@@ -62,18 +62,38 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do ...@@ -62,18 +62,38 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do
context 'in authorization headers' do context 'in authorization headers' do
let(:problematic_input) { null_byte } let(:problematic_input) { null_byte }
shared_examples 'rejecting invalid input' do
it 'rejects problematic input in the password' do it 'rejects problematic input in the password' do
env = env_for.merge(auth_env("username", "password#{problematic_input}encoded", nil)) env = env_for.merge(auth_env("username", "password#{problematic_input}encoded", nil))
expect(subject.call(env)).to eq error_400 expect(subject.call(env)).to eq error_400
end end
it 'rejects problematic input in the password' do it 'rejects problematic input in the username' do
env = env_for.merge(auth_env("username#{problematic_input}", "password#{problematic_input}encoded", nil)) env = env_for.merge(auth_env("username#{problematic_input}", "passwordencoded", nil))
expect(subject.call(env)).to eq error_400 expect(subject.call(env)).to eq error_400
end end
it 'rejects problematic input in non-basic-auth tokens' do
env = env_for.merge('HTTP_AUTHORIZATION' => "GL-Geo hello#{problematic_input}world")
expect(subject.call(env)).to eq error_400
end
end
it_behaves_like 'rejecting invalid input' do
let(:problematic_input) { null_byte }
end
it_behaves_like 'rejecting invalid input' do
let(:problematic_input) { invalid_string }
end
it_behaves_like 'rejecting invalid input' do
let(:problematic_input) { "\xC3" }
end
it 'does not reject correct non-basic-auth tokens' do it 'does not reject correct non-basic-auth tokens' do
# This token is known to include a null-byte when we were to try to decode it # This token is known to include a null-byte when we were to try to decode it
# as Base64, while it wasn't encoded at such. # as Base64, while it wasn't encoded at such.
...@@ -84,12 +104,6 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do ...@@ -84,12 +104,6 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do
expect(subject.call(env)).not_to eq error_400 expect(subject.call(env)).not_to eq error_400
end end
it 'rejects problematic input in non-basic-auth tokens' do
env = env_for.merge('HTTP_AUTHORIZATION' => "GL-Geo hello#{problematic_input}world")
expect(subject.call(env)).to eq error_400
end
end end
context 'in params' do context 'in params' 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