Skip to content

Commit 64c5045

Browse files
mamehsbt
authored andcommitted
Prevent CRLF injection
Throw a RuntimeError if the HTTP response header contains CR or LF to prevent HTTP response splitting. https://hackerone.com/reports/1204695
1 parent 30107a4 commit 64c5045

File tree

2 files changed

+36
-17
lines changed

2 files changed

+36
-17
lines changed

lib/cgi/core.rb

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,28 @@ def http_header(options='text/html')
188188
# Using #header with the HTML5 tag maker will create a <header> element.
189189
alias :header :http_header
190190

191+
def _no_crlf_check(str)
192+
if str
193+
str = str.to_s
194+
raise "A HTTP status or header field must not include CR and LF" if str =~ /[\r\n]/
195+
str
196+
else
197+
nil
198+
end
199+
end
200+
private :_no_crlf_check
201+
191202
def _header_for_string(content_type) #:nodoc:
192203
buf = ''.dup
193204
if nph?()
194-
buf << "#{$CGI_ENV['SERVER_PROTOCOL'] || 'HTTP/1.0'} 200 OK#{EOL}"
205+
buf << "#{_no_crlf_check($CGI_ENV['SERVER_PROTOCOL']) || 'HTTP/1.0'} 200 OK#{EOL}"
195206
buf << "Date: #{CGI.rfc1123_date(Time.now)}#{EOL}"
196-
buf << "Server: #{$CGI_ENV['SERVER_SOFTWARE']}#{EOL}"
207+
buf << "Server: #{_no_crlf_check($CGI_ENV['SERVER_SOFTWARE'])}#{EOL}"
197208
buf << "Connection: close#{EOL}"
198209
end
199-
buf << "Content-Type: #{content_type}#{EOL}"
210+
buf << "Content-Type: #{_no_crlf_check(content_type)}#{EOL}"
200211
if @output_cookies
201-
@output_cookies.each {|cookie| buf << "Set-Cookie: #{cookie}#{EOL}" }
212+
@output_cookies.each {|cookie| buf << "Set-Cookie: #{_no_crlf_check(cookie)}#{EOL}" }
202213
end
203214
return buf
204215
end # _header_for_string
@@ -213,48 +224,48 @@ def _header_for_hash(options) #:nodoc:
213224
## NPH
214225
options.delete('nph') if defined?(MOD_RUBY)
215226
if options.delete('nph') || nph?()
216-
protocol = $CGI_ENV['SERVER_PROTOCOL'] || 'HTTP/1.0'
227+
protocol = _no_crlf_check($CGI_ENV['SERVER_PROTOCOL']) || 'HTTP/1.0'
217228
status = options.delete('status')
218-
status = HTTP_STATUS[status] || status || '200 OK'
229+
status = HTTP_STATUS[status] || _no_crlf_check(status) || '200 OK'
219230
buf << "#{protocol} #{status}#{EOL}"
220231
buf << "Date: #{CGI.rfc1123_date(Time.now)}#{EOL}"
221232
options['server'] ||= $CGI_ENV['SERVER_SOFTWARE'] || ''
222233
options['connection'] ||= 'close'
223234
end
224235
## common headers
225236
status = options.delete('status')
226-
buf << "Status: #{HTTP_STATUS[status] || status}#{EOL}" if status
237+
buf << "Status: #{HTTP_STATUS[status] || _no_crlf_check(status)}#{EOL}" if status
227238
server = options.delete('server')
228-
buf << "Server: #{server}#{EOL}" if server
239+
buf << "Server: #{_no_crlf_check(server)}#{EOL}" if server
229240
connection = options.delete('connection')
230-
buf << "Connection: #{connection}#{EOL}" if connection
241+
buf << "Connection: #{_no_crlf_check(connection)}#{EOL}" if connection
231242
type = options.delete('type')
232-
buf << "Content-Type: #{type}#{EOL}" #if type
243+
buf << "Content-Type: #{_no_crlf_check(type)}#{EOL}" #if type
233244
length = options.delete('length')
234-
buf << "Content-Length: #{length}#{EOL}" if length
245+
buf << "Content-Length: #{_no_crlf_check(length)}#{EOL}" if length
235246
language = options.delete('language')
236-
buf << "Content-Language: #{language}#{EOL}" if language
247+
buf << "Content-Language: #{_no_crlf_check(language)}#{EOL}" if language
237248
expires = options.delete('expires')
238249
buf << "Expires: #{CGI.rfc1123_date(expires)}#{EOL}" if expires
239250
## cookie
240251
if cookie = options.delete('cookie')
241252
case cookie
242253
when String, Cookie
243-
buf << "Set-Cookie: #{cookie}#{EOL}"
254+
buf << "Set-Cookie: #{_no_crlf_check(cookie)}#{EOL}"
244255
when Array
245256
arr = cookie
246-
arr.each {|c| buf << "Set-Cookie: #{c}#{EOL}" }
257+
arr.each {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
247258
when Hash
248259
hash = cookie
249-
hash.each_value {|c| buf << "Set-Cookie: #{c}#{EOL}" }
260+
hash.each_value {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
250261
end
251262
end
252263
if @output_cookies
253-
@output_cookies.each {|c| buf << "Set-Cookie: #{c}#{EOL}" }
264+
@output_cookies.each {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
254265
end
255266
## other headers
256267
options.each do |key, value|
257-
buf << "#{key}: #{value}#{EOL}"
268+
buf << "#{_no_crlf_check(key)}: #{_no_crlf_check(value)}#{EOL}"
258269
end
259270
return buf
260271
end # _header_for_hash

test/cgi/test_cgi_header.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,14 @@ def test_cgi_http_header_nph
176176
end
177177

178178

179+
def test_cgi_http_header_crlf_injection
180+
cgi = CGI.new
181+
assert_raise(RuntimeError) { cgi.http_header("text/xhtml\r\nBOO") }
182+
assert_raise(RuntimeError) { cgi.http_header("type" => "text/xhtml\r\nBOO") }
183+
assert_raise(RuntimeError) { cgi.http_header("status" => "200 OK\r\nBOO") }
184+
assert_raise(RuntimeError) { cgi.http_header("location" => "text/xhtml\r\nBOO") }
185+
end
186+
179187

180188
instance_methods.each do |method|
181189
private method if method =~ /^test_(.*)/ && $1 != ENV['TEST']

0 commit comments

Comments
 (0)