Skip to content

Commit

Permalink
Validates response processed by exception handler
Browse files Browse the repository at this point in the history
If block of `rescue_from` return nil, the response would be nil.
That causes system information leak. Add a validation to prevent it.
  • Loading branch information
darren987469 committed Aug 5, 2018
1 parent fd53891 commit 52e9dea
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,13 @@ def run_rescue_handler(handler, error)
handler = public_method(handler)
end

handler.arity.zero? ? instance_exec(&handler) : instance_exec(error, &handler)
response = handler.arity.zero? ? instance_exec(&handler) : instance_exec(error, &handler)
valid_response?(response) ? response : error!('Internal Server Error(Invalid Response)')
end

def valid_response?(response)
# Rack::Response.new(...).finish generates an array with size 3
response.is_a?(Array) && response.size == 3
end
end
end
Expand Down
20 changes: 20 additions & 0 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1723,6 +1723,26 @@ class CustomError < Grape::Exceptions::Base; end
expect(last_response.status).to eql 500
expect(last_response.body).to eq('Formatter Error')
end

it 'validates response processed by exception handler' do
subject.rescue_from ArgumentError do
error!('rain!')
nil # invalid response caused by return nil
end
subject.rescue_from :all do
error!('rain!')
end
subject.get('/invalid_response') { raise ArgumentError }
subject.get('/valid_response') { raise 'rain!' }

get '/invalid_response'
expect(last_response.status).to eql 500
expect(last_response.body).to eq('Internal Server Error(Invalid Response)')

get '/valid_response'
expect(last_response.status).to eql 500
expect(last_response.body).to eq('rain!')
end
end

describe '.rescue_from klass, block' do
Expand Down

0 comments on commit 52e9dea

Please sign in to comment.