From dd24bf564b41ad3c244c388a8f01bc8614fcc40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Cournoyer?= Date: Sun, 29 Nov 2020 11:02:27 -0500 Subject: [PATCH 1/2] Fix possible HTTP Response Splitting Ignore any response header with \r or \n in their value See https://github.com/advisories/GHSA-84j7-475p-hp8v --- lib/thin/headers.rb | 3 ++- spec/headers_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/thin/headers.rb b/lib/thin/headers.rb index be9b576b..a33f50b2 100644 --- a/lib/thin/headers.rb +++ b/lib/thin/headers.rb @@ -4,6 +4,7 @@ module Thin class Headers HEADER_FORMAT = "%s: %s\r\n".freeze ALLOWED_DUPLICATES = %w(set-cookie set-cookie2 warning www-authenticate).freeze + CR_OR_LF = /[\r\n]/.freeze def initialize @sent = {} @@ -20,7 +21,7 @@ def []=(key, value) value = case value when Time value.httpdate - when NilClass + when NilClass, CR_OR_LF return else value.to_s diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index 4a112dc2..6c2433d6 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -37,4 +37,24 @@ @headers['Modified-At'] = time expect(@headers.to_s).to include("Modified-At: #{time.httpdate}") end + + it 'should format Integer values correctly' do + @headers['X-Number'] = 32 + expect(@headers.to_s).to include("X-Number: 32") + end + + it 'should not allow CRLF' do + @headers['Bad'] = "a\r\nSet-Cookie: injected=value" + expect(@headers.to_s).to be_empty + end + + it 'should not allow CR' do + @headers['Bad'] = "a\rSet-Cookie: injected=value" + expect(@headers.to_s).to be_empty + end + + it 'should not allow LF' do + @headers['Bad'] = "a\nSet-Cookie: injected=value" + expect(@headers.to_s).to be_empty + end end \ No newline at end of file From 6c58953a0f841ec78a5409d086ee03f7d3d8a0e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Cournoyer?= Date: Tue, 18 May 2021 16:17:26 -0400 Subject: [PATCH 2/2] Raise an exception when a response header contains CR or LF --- lib/thin/headers.rb | 8 +++++++- spec/headers_spec.rb | 9 +++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/thin/headers.rb b/lib/thin/headers.rb index a33f50b2..4f773b44 100644 --- a/lib/thin/headers.rb +++ b/lib/thin/headers.rb @@ -1,4 +1,8 @@ module Thin + # Raised when an header is not valid + # and the server can not process it. + class InvalidHeader < StandardError; end + # Store HTTP header name-value pairs direcly to a string # and allow duplicated entries on some names. class Headers @@ -21,8 +25,10 @@ def []=(key, value) value = case value when Time value.httpdate - when NilClass, CR_OR_LF + when NilClass return + when CR_OR_LF + raise InvalidHeader, "Header contains CR or LF" else value.to_s end diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index 6c2433d6..2ef0fe6e 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -44,17 +44,14 @@ end it 'should not allow CRLF' do - @headers['Bad'] = "a\r\nSet-Cookie: injected=value" - expect(@headers.to_s).to be_empty + expect { @headers['Bad'] = "a\r\nSet-Cookie: injected=value" }.to raise_error(InvalidHeader) end it 'should not allow CR' do - @headers['Bad'] = "a\rSet-Cookie: injected=value" - expect(@headers.to_s).to be_empty + expect { @headers['Bad'] = "a\rSet-Cookie: injected=value" }.to raise_error(InvalidHeader) end it 'should not allow LF' do - @headers['Bad'] = "a\nSet-Cookie: injected=value" - expect(@headers.to_s).to be_empty + expect { @headers['Bad'] = "a\nSet-Cookie: injected=value" }.to raise_error(InvalidHeader) end end \ No newline at end of file