Default serializer will use YAML.safe_load

Appraisals

@@ -26,6 +26,6 @@ appraise "rails-6.1" do
 end
 
 appraise "rails-7.0" do
-  gem "rails", "~> 7.0.0"
+  gem "rails", "~> 7.0.3.1"
   gem "rails-controller-testing", "~> 1.0.5"
 end

CHANGELOG.md

@@ -17,6 +17,25 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 - None
 
+## 13.0.0 (2022-08-15)
+
+### Breaking Changes
+
+- The default serializer will now use `YAML.safe_load` unless
+  `ActiveRecord.use_yaml_unsafe_load`. This change only affects users whose
+  `versions` table has `object` or `object_changes` columns of type `text`, and
+  who use the YAML serializer. People who use the JSON serializer, or those with
+  `json(b)` columns, are unaffected. Please see [doc/pt_13_yaml_safe_load.md] for
+  details.
+
+### Added
+
+- None
+
+### Fixed
+
+- None
+
 ## 12.3.0 (2022-03-13)
 
 ### Breaking Changes

doc/pt_13_yaml_safe_load.md

@@ -0,0 +1,49 @@
+# PT 13 uses YAML.safe_load
+
+Starting with 13.0.0, PT's default serializer (`PaperTrail::Serializers::YAML`)
+will use `safe_load` unless `ActiveRecord.use_yaml_unsafe_load`. Earlier
+versions of PT use `unsafe_load`.
+
+## Motivation
+
+> A few days ago Rails released versions 7.0.3.1, 6.1.6.1, 6.0.5.1, and 5.2.8.1.
+> These are security updates that impact applications that use serialised
+> attributes on Active Record models. These updates, identified by CVE-2022-32224
+> cover a possible escalation to RCE when using YAML serialised columns in Active
+> Record.
+> https://rubyonrails.org/2022/7/15/this-week-in-rails-rails-security-releases-improved-generator-option-handling-and-more-24774592
+
+## Who is affected by this change?
+
+This change only affects users whose `versions` table has `object` or
+`object_changes` columns of type `text`, and who use the YAML serializer. People
+who use the JSON serializer, or those with `json(b)` columns, are unaffected.
+
+## To continue using the YAML serializer
+
+We recommend switching to `json(b)` columns, or at least JSON in a `text` column
+(see "Other serializers" below). If you must continue using the YAML serializer,
+PT users are required to configure `ActiveRecord.yaml_column_permitted_classes`
+correctly for their own application. Users may want to start with the following
+safe-list:
+
+```ruby
+::ActiveRecord.use_yaml_unsafe_load = false
+::ActiveRecord.yaml_column_permitted_classes = [
+  ::ActiveRecord::Type::Time::Value,
+  ::ActiveSupport::TimeWithZone,
+  ::ActiveSupport::TimeZone,
+  ::BigDecimal,
+  ::Date,
+  ::Symbol,
+  ::Time
+]
+```
+
+## Other serializers
+
+While YAML remains the default serializer in PT for historical compatibility,
+we have recommended JSON instead, for years. See:
+
+- [PostgreSQL JSON column type support](https://github.com/paper-trail-gem/paper_trail/blob/v12.3.0/README.md#postgresql-json-column-type-support)
+- [Convert existing YAML data to JSON](https://github.com/paper-trail-gem/paper_trail/blob/v12.3.0/README.md#convert-existing-yaml-data-to-json)

gemfiles/rails_7.0.gemfile

@@ -2,7 +2,7 @@
 
 source "https://rubygems.org"
 
-gem "rails", "~> 7.0.0"
+gem "rails", "~> 7.0.3.1"
 gem "rails-controller-testing", "~> 1.0.5"
 
 gemspec path: "../"

lib/paper_trail/serializers/yaml.rb

@@ -9,7 +9,17 @@ module YAML
       extend self # makes all instance methods become module methods as well
 
       def load(string)
-        ::YAML.respond_to?(:unsafe_load) ? ::YAML.unsafe_load(string) : ::YAML.load(string)
+        if use_safe_load?
+          ::YAML.safe_load(
+            string,
+            permitted_classes: ::ActiveRecord.yaml_column_permitted_classes,
+            aliases: true
+          )
+        elsif ::YAML.respond_to?(:unsafe_load)
+          ::YAML.unsafe_load(string)
+        else
+          ::YAML.load(string)
+        end
       end
 
       # @param object (Hash | HashWithIndifferentAccess) - Coming from
@@ -26,6 +36,14 @@ def dump(object)
       def where_object_condition(arel_field, field, value)
         arel_field.matches("%\n#{field}: #{value}\n%")
       end
+
+      private
+
+      # `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
+      def use_safe_load?
+        defined?(ActiveRecord.use_yaml_unsafe_load) &&
+          !ActiveRecord.use_yaml_unsafe_load
+      end
     end
   end
 end

lib/paper_trail/version_concern.rb

@@ -15,6 +15,12 @@ module PaperTrail
   module VersionConcern
     extend ::ActiveSupport::Concern
 
+    E_YAML_PERMITTED_CLASSES = <<-EOS.squish.freeze
+      PaperTrail encountered a Psych::DisallowedClass error during
+      deserialization of YAML column, indicating that
+      yaml_column_permitted_classes has not been configured correctly. %s
+    EOS
+
     included do
       belongs_to :item, polymorphic: true, optional: true, inverse_of: false
       validates_presence_of :event
@@ -348,7 +354,10 @@ def object_changes_deserialized
       else
         begin
           PaperTrail.serializer.load(object_changes)
-        rescue StandardError # TODO: Rescue something more specific
+        rescue StandardError => e
+          if defined?(::Psych::Exception) && e.instance_of?(::Psych::Exception)
+            ::Kernel.warn format(E_YAML_PERMITTED_CLASSES, e)
+          end
           {}
         end
       end

spec/dummy_app/config/application.rb

@@ -27,5 +27,19 @@ class Application < Rails::Application
         ::Gem::Requirement.new("~> 5.2").satisfied_by?(::Rails.gem_version)
       config.active_record.sqlite3.represent_boolean_as_integer = true
     end
+
+    # `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
+    if ::ActiveRecord.respond_to?(:use_yaml_unsafe_load)
+      ::ActiveRecord.use_yaml_unsafe_load = false
+      ::ActiveRecord.yaml_column_permitted_classes = [
+        ::ActiveRecord::Type::Time::Value,
+        ::ActiveSupport::TimeWithZone,
+        ::ActiveSupport::TimeZone,
+        ::BigDecimal,
+        ::Date,
+        ::Symbol,
+        ::Time
+      ]
+    end
   end
 end

spec/models/widget_spec.rb

@@ -46,6 +46,21 @@
     end
   end
 
+  describe "#object_changes_deserialized" do
+    context "when the serializer raises a Psych::DisallowedClass error" do
+      it "prints a warning to stderr" do
+        allow(PaperTrail.serializer).to(
+          receive(:load).and_raise(::Psych::Exception, "kaboom")
+        )
+        widget = described_class.create(name: "Henry")
+        ver = widget.versions.last
+        expect { ver.send(:object_changes_deserialized) }.to(
+          output(/kaboom/).to_stderr
+        )
+      end
+    end
+  end
+
   context "with a new record" do
     it "not have any previous versions" do
       expect(described_class.new.versions).to(eq([]))

spec/paper_trail/serializers/yaml_spec.rb

@@ -24,8 +24,13 @@ module Serializers
         end
 
         it "calls the expected load method based on Psych version" do
+          # `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
+          if defined?(ActiveRecord.use_yaml_unsafe_load) && !ActiveRecord.use_yaml_unsafe_load
+            allow(::YAML).to receive(:safe_load)
+            described_class.load("string")
+            expect(::YAML).to have_received(:safe_load)
           # Psych 4+ implements .unsafe_load
-          if ::YAML.respond_to?(:unsafe_load)
+          elsif ::YAML.respond_to?(:unsafe_load)
             allow(::YAML).to receive(:unsafe_load)
             described_class.load("string")
             expect(::YAML).to have_received(:unsafe_load)

spec/spec_helper.rb

@@ -5,7 +5,7 @@
 
 require "simplecov"
 SimpleCov.start do
-  add_filter "spec"
+  add_filter %w[Appraisals Gemfile Rakefile doc gemfiles spec]
 end
 SimpleCov.minimum_coverage(ENV["DB"] == "postgres" ? 97.3 : 92.4)