From cbbe3a808b4832fb875a9ea5b137d268f7ecde7b Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 2 Oct 2023 11:41:34 +0800 Subject: [PATCH] SECURITY: Add a default limit as to when logs should be truncated Why this change? This ensures that malicious requests cannot end up causing the logs to quickly fill up. The default chosen is sufficient for most legitimate requests to the Discourse application. When truncation happens, parsing of logs in supported format like lograge may break down. --- config/discourse_defaults.conf | 3 +++ config/initializers/102-truncate-logs.rb | 31 ++++++++++++++++++++++++ lib/truncate_logs_formatter.rb | 29 ++++++++++++++++++++++ spec/lib/truncate_logs_formatter_spec.rb | 31 ++++++++++++++++++++++++ 4 files changed, 94 insertions(+) create mode 100644 config/initializers/102-truncate-logs.rb create mode 100644 lib/truncate_logs_formatter.rb create mode 100644 spec/lib/truncate_logs_formatter_spec.rb diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index fabfeccd5d7..b2d96e4bde0 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -374,3 +374,6 @@ regex_timeout_seconds = 2 # Allow impersonation function on the cluster to admins allow_impersonation = true + +# The maximum number of characters allowed in a single log line. +log_line_max_chars = 160000 diff --git a/config/initializers/102-truncate-logs.rb b/config/initializers/102-truncate-logs.rb new file mode 100644 index 00000000000..b1276f3d967 --- /dev/null +++ b/config/initializers/102-truncate-logs.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +if Rails.env.production? || ENV["ENABLE_LOGS_TRUNCATION"] == "1" + def set_or_extend_truncate_logs_formatter(logger) + if logger.formatter + logger.formatter.extend( + Module.new do + def call(*args) + truncate_logs_formatter.call(super(*args)) + end + + def truncate_logs_formatter + @formatter ||= + TruncateLogsFormatter.new(log_line_max_chars: GlobalSetting.log_line_max_chars) + end + end, + ) + else + logger.formatter = + TruncateLogsFormatter.new(log_line_max_chars: GlobalSetting.log_line_max_chars) + end + end + + Rails.application.config.to_prepare do + set_or_extend_truncate_logs_formatter(Rails.logger) + + if Rails.logger.respond_to? :chained + Rails.logger.chained.each { |logger| set_or_extend_truncate_logs_formatter(logger) } + end + end +end diff --git a/lib/truncate_logs_formatter.rb b/lib/truncate_logs_formatter.rb new file mode 100644 index 00000000000..b22f913a186 --- /dev/null +++ b/lib/truncate_logs_formatter.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# This log formatter limits the number of characters in each log message to prevent malicious requests from filling up the disk +# in a short amount of time. The number of characters is determined by the `log_line_max_chars` global setting which can be +# configured via the `DISCOURSE_MAX_LOG_LINES` environment variable or via the `discourse_defaults.conf` file. +class TruncateLogsFormatter < ::ActiveSupport::Logger::SimpleFormatter + include ::ActiveSupport::TaggedLogging::Formatter + + def initialize(log_line_max_chars:) + @log_line_max_chars = log_line_max_chars + end + + def call(*args) + # Lograge formatters are only called with a single argument instead of the usual 4 arguments of `severity`, `datetime`, `progname` and `message`. + message = + if args.length == 1 + args[0] + else + args[3] + end + + if message.length > @log_line_max_chars + newlines = message.length - message.chomp.length + "#{message[0, @log_line_max_chars]}...(truncated)#{"\n" * newlines}" + else + message + end + end +end diff --git a/spec/lib/truncate_logs_formatter_spec.rb b/spec/lib/truncate_logs_formatter_spec.rb new file mode 100644 index 00000000000..c1df6ea0f86 --- /dev/null +++ b/spec/lib/truncate_logs_formatter_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +RSpec.describe TruncateLogsFormatter do + describe "#call" do + describe "when the formatter is initialized with `log_line_max_chars` of 10" do + let(:formatter) { TruncateLogsFormatter.new(log_line_max_chars: 10) } + + describe "when the messages is 5 characters long" do + it "should not carry out any truncation of the message" do + expect(formatter.call(nil, nil, nil, "abcde")).to eq("abcde") + end + end + + describe "when the message is 10 characters long" do + it "should not carry out any truncation of the message" do + expect(formatter.call(nil, nil, nil, "aaaaaaaaaa")).to eq("aaaaaaaaaa") + end + end + + describe "when the message is 11 characters long" do + it "should truncate the message with the right postfix" do + expect(formatter.call(nil, nil, nil, "aaaaaaaaaaa")).to eq("aaaaaaaaaa...(truncated)") + end + + it "should truncate the message with the right postfix while preserving newlines" do + expect(formatter.call(nil, nil, nil, "aaaaaaaaaaa\n")).to eq("aaaaaaaaaa...(truncated)\n") + end + end + end + end +end