Skip to content

Commit

Permalink
Rescue exceptions raised when calling to_s while sanitizing
Browse files Browse the repository at this point in the history
In the case that calling to_s will raise, like in
ActionDispatch::RemoteIp::IpSpoofAttackError, rescue and insert a
placeholder.

Ref: 
https://github.com/rails/rails/blob/34fe2a4fc778d18b7fe6bdf3629c1481bee789b9/actionpack/lib/action_dispatch/middleware/remote_ip.rb#L134
  • Loading branch information
kattrali committed Jun 13, 2017
1 parent 8890e26 commit 758a030
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/bugsnag/cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Cleaner
FILTERED = '[FILTERED]'.freeze
RECURSION = '[RECURSION]'.freeze
OBJECT = '[OBJECT]'.freeze
SPOOF = '[SPOOF]'.freeze

def initialize(filters)
@filters = Array(filters)
Expand Down Expand Up @@ -43,7 +44,7 @@ def traverse_object(obj, seen, scope)
when String
clean_string(obj)
else
str = obj.to_s
str = obj.to_s rescue SPOOF
# avoid leaking potentially sensitive data from objects' #inspect output
if str =~ /#<.*>/
OBJECT
Expand Down
9 changes: 9 additions & 0 deletions spec/cleaner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@
params = {:request => {:params => {:foo => {:bar => "baz"}}}}
expect(described_class.new([/^foo\.bar/]).clean_object(params)).to eq({:request => {:params => {:foo => {:bar => '[FILTERED]'}}}})
end

it "filters objects which can't be stringified" do
class StringRaiser
def to_s
raise 'Oh no you do not!'
end
end
expect(subject.clean_object({ :foo => StringRaiser.new })).to eq({ :foo => '[SPOOF]' })
end
end

describe "#clean_url" do
Expand Down
13 changes: 13 additions & 0 deletions spec/helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@
expect(value[4]).to be_a(String)
end

context "an object will throw if `to_s` is called" do
class StringRaiser
def to_s
raise 'Oh no you do not!'
end
end

it "uses the string '[SPOOF]' instead" do
value = Bugsnag::Helpers.trim_if_needed([1, 3, StringRaiser.new])
expect(value[2]).to eq "[SPOOF]"
end
end

context "payload length is less than allowed" do

it "does not change strings" do
Expand Down

0 comments on commit 758a030

Please sign in to comment.