github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-25 09:42:07 +08:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

SECURITY: Onebox response timeout and size limit (#15927)

Browse Source 
Validation to ensure that Onebox request is no longer than 10 seconds and response size is not bigger than 1 MB
This commit is contained in:
Krzysztof Kotlarek 2022-02-14 12:11:09 +11:00 committed by GitHub
parent 55007fbf55
commit a34075d205
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
lib/final_destination.rb
View File

@ -8,6 +8,8 @@ require 'url_helper'
# Determine the final endpoint for a Web URI, following redirects # Determine the final endpoint for a Web URI, following redirects
class FinalDestination class FinalDestination
MAX_REQUEST_TIME_SECONDS = 10
MAX_REQUEST_SIZE_BYTES = 1_048_576 # 1024 * 1024
def self.clear_https_cache!(domain) def self.clear_https_cache!(domain)
key = redis_https_key(domain) key = redis_https_key(domain)
@ -203,12 +205,21 @@ class FinalDestination
middlewares = Excon.defaults[:middlewares] middlewares = Excon.defaults[:middlewares]
middlewares << Excon::Middleware::Decompress if @http_verb == :get middlewares << Excon::Middleware::Decompress if @http_verb == :get
request_start_time = Time.now
response_body = +""
request_validator = lambda do |chunk, _remaining_bytes, _total_bytes|
response_body << chunk
raise Excon::Errors::ExpectationFailed.new("response size too big: #{@uri.to_s}") if response_body.bytesize > MAX_REQUEST_SIZE_BYTES
raise Excon::Errors::ExpectationFailed.new("connect timeout reached: #{@uri.to_s}") if Time.now - request_start_time > MAX_REQUEST_TIME_SECONDS
end
response = Excon.public_send(@http_verb, response = Excon.public_send(@http_verb,
@uri.to_s, @uri.to_s,
read_timeout: timeout, read_timeout: timeout,
connect_timeout: timeout, connect_timeout: timeout,
headers: headers, headers: headers,
middlewares: middlewares middlewares: middlewares,
response_block: request_validator
) )
location = nil location = nil
@ -220,12 +231,12 @@ class FinalDestination
# Cache body of successful `get` requests # Cache body of successful `get` requests
if @http_verb == :get if @http_verb == :get
if Oneboxer.cache_response_body?(@uri) if Oneboxer.cache_response_body?(@uri)
Oneboxer.cache_response_body(@uri.to_s, response.body) Oneboxer.cache_response_body(@uri.to_s, response_body)
end end
end end
if @follow_canonical if @follow_canonical
next_url = fetch_canonical_url(response.body) next_url = fetch_canonical_url(response_body)
if next_url.to_s.present? && next_url != @uri if next_url.to_s.present? && next_url != @uri
@follow_canonical = false @follow_canonical = false

21
spec/components/final_destination_spec.rb
View File

@ -49,6 +49,13 @@ describe FinalDestination do
} }
end end
let(:body_response) do
{
status: 200,
body: "<body>test</body>"
}
end
def canonical_follow(from, dest) def canonical_follow(from, dest)
stub_request(:get, from).to_return( stub_request(:get, from).to_return(
status: 200, status: 200,
@ -182,6 +189,20 @@ describe FinalDestination do
end end
end end
it 'raises error when response is too big' do
stub_const(described_class, "MAX_REQUEST_SIZE_BYTES", 1) do
stub_request(:get, "https://codinghorror.com/blog").to_return(body_response)
final = FinalDestination.new('https://codinghorror.com/blog', opts.merge(follow_canonical: true))
expect { final.resolve }.to raise_error(Excon::Errors::ExpectationFailed, "response size too big: https://codinghorror.com/blog")
end
end
it 'raises error when response is too slow' do
stub_request(:get, "https://codinghorror.com/blog").to_return(lambda { |request| freeze_time(11.seconds.from_now) ; body_response })
final = FinalDestination.new('https://codinghorror.com/blog', opts.merge(follow_canonical: true))
expect { final.resolve }.to raise_error(Excon::Errors::ExpectationFailed, "connect timeout reached: https://codinghorror.com/blog")
end
context 'follows canonical links' do context 'follows canonical links' do
it 'resolves the canonical link as the final destination' do it 'resolves the canonical link as the final destination' do
canonical_follow("https://eviltrout.com", "https://codinghorror.com/blog") canonical_follow("https://eviltrout.com", "https://codinghorror.com/blog")