Skip to content
This repository
Browse code

Remove the use of String#% when formatting durations in log messages

This avoids potential format string vulnerabilities where user-provided
data is interpolated into the log message before String#% is called.
  • Loading branch information...
commit 5aee516b5edb49d7206cd9815c13a78b6b16c5d9 1 parent 54c05ac
Michael Koziarski NZKoz authored tenderlove committed
6 actionmailer/lib/action_mailer/log_subscriber.rb
@@ -4,12 +4,12 @@ module ActionMailer
4 4 class LogSubscriber < ActiveSupport::LogSubscriber
5 5 def deliver(event)
6 6 recipients = Array.wrap(event.payload[:to]).join(', ')
7   - info("\nSent mail to #{recipients} (%1.fms)" % event.duration)
  7 + info("\nSent mail to #{recipients} (#{format_duration(event.duration)})")
8 8 debug(event.payload[:mail])
9 9 end
10 10
11 11 def receive(event)
12   - info("\nReceived mail (%.1fms)" % event.duration)
  12 + info("\nReceived mail (#{format_duration(event.duration)})")
13 13 debug(event.payload[:mail])
14 14 end
15 15
@@ -19,4 +19,4 @@ def logger
19 19 end
20 20 end
21 21
22   -ActionMailer::LogSubscriber.attach_to :action_mailer
  22 +ActionMailer::LogSubscriber.attach_to :action_mailer
11 actionpack/lib/action_controller/log_subscriber.rb
@@ -23,7 +23,7 @@ def process_action(event)
23 23 exception_class_name = payload[:exception].first
24 24 status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
25 25 end
26   - message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration
  26 + message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{format_duration(event.duration)}"
27 27 message << " (#{additions.join(" | ")})" unless additions.blank?
28 28
29 29 info(message)
@@ -34,9 +34,7 @@ def halted_callback(event)
34 34 end
35 35
36 36 def send_file(event)
37   - message = "Sent file %s"
38   - message << " (%.1fms)"
39   - info(message % [event.payload[:path], event.duration])
  37 + info("Sent file #{event.payload[:path]} (#{format_duration(event.duration)})")
40 38 end
41 39
42 40 def redirect_to(event)
@@ -44,7 +42,7 @@ def redirect_to(event)
44 42 end
45 43
46 44 def send_data(event)
47   - info("Sent data %s (%.1fms)" % [event.payload[:filename], event.duration])
  45 + info("Sent data #{event.payload[:filename]} (#{format_duration(event.duration)})")
48 46 end
49 47
50 48 %w(write_fragment read_fragment exist_fragment?
@@ -53,7 +51,8 @@ def send_data(event)
53 51 def #{method}(event)
54 52 key_or_path = event.payload[:key] || event.payload[:path]
55 53 human_name = #{method.to_s.humanize.inspect}
56   - info("\#{human_name} \#{key_or_path} \#{"(%.1fms)" % event.duration}")
  54 + duration = format_duration(event.duration)
  55 + info("\#{human_name} \#{key_or_path} \#{duration}")
57 56 end
58 57 METHOD
59 58 end
4 activesupport/lib/active_support/log_subscriber.rb
@@ -118,5 +118,9 @@ def color(text, color, bold=false)
118 118 bold = bold ? BOLD : ""
119 119 "#{bold}#{color}#{text}#{CLEAR}"
120 120 end
  121 +
  122 + def format_duration(duration)
  123 + "%.1fms" % duration
  124 + end
121 125 end
122 126 end

0 comments on commit 5aee516

Please sign in to comment.
Something went wrong with that request. Please try again.