Default serializer will use YAML.safe_load

Breaking change.

Going forward, PT's default serializer (PaperTrail::Serializers::YAML)
will use `safe_load` unless `ActiveRecord.use_yaml_unsafe_load`.

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
]
```
This commit is contained in:
Jared Beck 2022-07-27 02:46:30 -04:00
parent f7462ea7ea
commit d17fdabd29
10 changed files with 135 additions and 6 deletions

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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: "../"

View File

@ -9,7 +9,17 @@ module PaperTrail
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 @@ module PaperTrail
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

View File

@ -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 @@ module PaperTrail
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

View File

@ -27,5 +27,19 @@ module Dummy
::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

View File

@ -46,6 +46,21 @@ RSpec.describe Widget, type: :model, versioning: true do
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([]))

View File

@ -24,8 +24,13 @@ module PaperTrail
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)

View File

@ -5,7 +5,7 @@ ENV["DB"] ||= "sqlite"
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)