Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support CVE-2022-32224 Rails security updates - backport to v3.1 #4453

Merged
merged 9 commits into from
Jul 15, 2022
Merged

Support CVE-2022-32224 Rails security updates - backport to v3.1 #4453

merged 9 commits into from
Jul 15, 2022

Conversation

gsmendoza
Copy link
Contributor

@gsmendoza gsmendoza commented Jul 15, 2022 •
edited
Loading

Summary

Rails Versions 7.0.3.1, 6.1.6.1, 6.0.5.1, and 5.2.8.1 have been released to address CVE-2022-32224, documented at https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017.

Currently, Solidus fails with the "Psych::DisallowedClass: Tried to load unspecified class: Symbol" error on those Rails versions. See https://app.circleci.com/pipelines/github/gsmendoza/solidus/14/workflows/88f440b9-3887-45c0-a013-593379d56ee5/jobs/85/steps.

References

Original solution

Backports

Additional notes

This PR backports YAML.safe_load changes to Spree::LogEntry.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • N/A: I have updated Guides and README accordingly to this change (if needed)
  • N/A: I have added tests to cover this change (if needed)
  • N/A: I have attached screenshots to this PR for visual changes (if needed)

gsmendoza and others added 8 commits July 14, 2022 18:18
@gsmendoza
Require psych to be >= 3.1.0
08ceaeb 
The permitted_classes keyword argument was introduced to psych starting
from psych 3.1.0. See
ruby/psych@682abf2.

Ruby 2.5.9 installs 3.0.2 by default.

This fixes errors like this when running the specs:

```
  1416) Taxation system integration tests selling from new york to new york an order with a download and a shipment has a shipment with additional tax of 0.40
        Failure/Error: preferences[name] = value

        ArgumentError:
          unknown keywords: permitted_classes, aliases
        # ./lib/spree/preferences/preferable_class_methods.rb:86:in `block in preference'
        # ./lib/spree/testing_support/factories/shipping_method_factory.rb:29:in `block (3 levels) in <top (required)>'
        # ./spec/models/spree/tax/taxation_integration_spec.rb:597:in `block (3 levels) in <top (required)>'
```
@waiting-for-dev @gsmendoza
Fix compatibility with Ruby 3.1' Psych version
df21741 
The new Psych version required by Ruby 3.1 no longer allows the loading
of arbitrary classes into YAML by default. We switch to `#safe_load` and
enable the class we're currently using.

See:

ruby/psych@cb50aa8
ruby/psych@1764942

[skip ci]
@waiting-for-dev @gsmendoza
Add missing class and allow extending serializable classes on logs
09bc67c 
`Spree::LogEntry` loads details from a YAML string that can potentially
contain any serialized Ruby object if users have overridden some part of
the codebase.

New Psych (YAML parser) version packed with Ruby 3.1 requires that, by
default, users are explicit about which type of classes are allowed to
be loaded (see ruby/psych@cb50aa8 and ruby/psych@1764942).

On 008168c, we updated our code to allow instances of
`ActiveMerchant::Billing::Response`. However, as [noted in a
comment](solidusio@008168c#r73483078),
it seems we're missing, at least, instances of
`ActiveSupport::TimeWithZone`.

We need to add all classes used internally and leave a door open for
users to add their own classes (via a new `log_entry_permitted_classes`
preference).
@waiting-for-dev @gsmendoza
Fix parsing YAML aliases in Spree::LogEntry
694f528 
By default, `YAML.safe_load` won't allow YAML aliases to be parsed.
However, we need them at least for older versions of serialized
`ActiveSupport::TimeWithZone`.

We add a configuration option so that users can still disable it.
Depending on the source of the YAML, parsing aliases could open gates to
entity expansion attacks (a DoS created by nesting self-references).
@gsmendoza
Permit Symbol for safe load
dc773fa 
Fixes "Psych::DisallowedClass: Tried to load unspecified class: Symbol"
errors when running specs.

See
https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017.
@gsmendoza
Permit BigDecimal for safe load
b934eef 
BigDecimals are persisted as preferences in Core e.g.

```
  1) Api Feature Specs is able to checkout with individualized requests
     Failure/Error: shipping_method.calculator.set_preference(:amount, 10.0)

     Psych::DisallowedClass:
       Tried to load unspecified class: BigDecimal
     # /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/preferable_class_methods.rb:86:in `block in preference'
     # /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/preferable.rb:79:in `set_preference'
     # ./spec/features/checkout_spec.rb:21:in `block (3 levels) in <module:Spree>'
     # ./spec/features/checkout_spec.rb:19:in `tap'
     # ./spec/features/checkout_spec.rb:19:in `block (2 levels) in <module:Spree>'
```

Here are the affected specs:

```
Failed examples:

rspec ./spec/features/checkout_spec.rb:118 # Api Feature Specs is able to checkout with individualized requests
rspec ./spec/features/checkout_spec.rb:137 # Api Feature Specs is able to checkout with the create request
rspec ./spec/features/checkout_spec.rb:164 # Api Feature Specs is able to checkout with the update request
rspec ./spec/requests/spree/api/checkouts_spec.rb:126 # Checkouts PUT 'update' can update shipping method and transition from delivery to payment
rspec ./spec/requests/spree/api/checkouts_spec.rb:99 # Checkouts PUT 'update' transitioning to delivery can update addresses but not transition to delivery w/o shipping setup
rspec ./spec/requests/spree/api/checkouts_spec.rb:374 # Checkouts PUT 'next' cannot transition if order email is blank
rspec ./spec/requests/spree/api/checkouts_spec.rb:443 # Checkouts PUT 'advance' returns the order
rspec ./spec/requests/spree/api/coupon_codes_spec.rb:28 # Coupon codes #create when successful applies the coupon
rspec ./spec/requests/spree/api/coupon_codes_spec.rb:77 # Coupon codes #destroy when successful removes the coupon
rspec ./spec/requests/spree/api/coupon_codes_spec.rb:90 # Coupon codes #destroy when unsuccessful returns an error
rspec ./spec/requests/spree/api/orders_spec.rb:87 # Orders POST create when the current user cannot administrate the order with existing promotion activates the promotion
rspec ./spec/requests/spree/api/orders_spec.rb:712 # Orders working with an order with a line item when in delivery includes the ship_total in the response
rspec ./spec/requests/spree/api/orders_spec.rb:719 # Orders working with an order with a line item when in delivery returns available shipments for an order
rspec ./spec/requests/spree/api/promotion_application_spec.rb:21 # Promotion application with an available promotion can apply a coupon code to the order
rspec ./spec/requests/spree/api/promotion_application_spec.rb:39 # Promotion application with an available promotion with an expired promotion fails to apply
rspec ./spec/requests/spree/api/shipments_spec.rb:326 # Shipments #estimated_rates returns success
rspec ./spec/requests/spree/api/shipments_spec.rb:331 # Shipments #estimated_rates returns rates available to user
rspec ./spec/requests/spree/api/shipments_spec.rb:344 # Shipments #estimated_rates returns rates available to admin
rspec ./spec/requests/spree/api/shipments_spec.rb:487 # Shipments transfers POST /api/shipments/transfer_to_location for a successful transfer returns the correct message
rspec ./spec/requests/spree/api/shipments_spec.rb:576 # Shipments transfers POST /api/shipments/transfer_to_shipment for a successful transfer returns the correct message
```

See
https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017.
@gsmendoza
Permit ActiveSupport::HashWithIndifferentAccess for safe load
fc0cdc7 
Spree::PromotionRule accepts ActiveSupport::HashWithIndifferentAccess as
persistable preferences.

See https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017

Affected backend specs:

```
Failures:

  1) Promotion with option value rule adding an option value rule
     Failure/Error: existing = promotion.rules.map { |rule| rule.class.name }

     ActionView::Template::Error:
       Tried to load unspecified class: ActiveSupport::HashWithIndifferentAccess
     # /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/persistable.rb:18:in `initialize_preference_defaults'
     # ./app/helpers/spree/promotion_rules_helper.rb:6:in `map'
     # ./app/helpers/spree/promotion_rules_helper.rb:6:in `options_for_promotion_rule_types'
     # ./app/views/spree/admin/promotions/_rules.html.erb:10:in `block in __home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions__rules_html_erb___2684429756418516206_286660'
     # ./app/views/spree/admin/promotions/_rules.html.erb:3:in `__home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions__rules_html_erb___2684429756418516206_286660'
     # ./app/views/spree/admin/promotions/edit.html.erb:29:in `__home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions_edit_html_erb___1754085209430652883_286520'
     # ./app/controllers/spree/admin/resource_controller.rb:27:in `block (2 levels) in edit'
     # ./app/controllers/spree/admin/resource_controller.rb:26:in `edit'
     # ------------------
     # --- Caused by: ---
     # Psych::DisallowedClass:
     #   Tried to load unspecified class: ActiveSupport::HashWithIndifferentAccess
     #   /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/persistable.rb:18:in `initialize_preference_defaults'

  2) Promotion with option value rule with an existing option value rule deleting a product
     Failure/Error: existing = promotion.rules.map { |rule| rule.class.name }

     ActionView::Template::Error:
       Tried to load unspecified class: ActiveSupport::HashWithIndifferentAccess
     # /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/persistable.rb:18:in `initialize_preference_defaults'
     # ./app/helpers/spree/promotion_rules_helper.rb:6:in `map'
     # ./app/helpers/spree/promotion_rules_helper.rb:6:in `options_for_promotion_rule_types'
     # ./app/views/spree/admin/promotions/_rules.html.erb:10:in `block in __home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions__rules_html_erb___2684429756418516206_286660'
     # ./app/views/spree/admin/promotions/_rules.html.erb:3:in `__home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions__rules_html_erb___2684429756418516206_286660'
     # ./app/views/spree/admin/promotions/edit.html.erb:29:in `__home_gsmendoza_repos_solidusio_solidus_backend_app_views_spree_admin_promotions_edit_html_erb___1754085209430652883_286520'
     # ./app/controllers/spree/admin/resource_controller.rb:27:in `block (2 levels) in edit'
     # ./app/controllers/spree/admin/resource_controller.rb:26:in `edit'
     # ------------------
     # --- Caused by: ---
     # Psych::DisallowedClass:
     #   Tried to load unspecified class: ActiveSupport::HashWithIndifferentAccess
     #   /home/gsmendoza/repos/solidusio/solidus/core/lib/spree/preferences/persistable.rb:18:in `initialize_preference_defaults'

Finished in 3 minutes 34.5 seconds (files took 2.87 seconds to load)
779 examples, 2 failures, 1 pending

Failed examples:

rspec ./spec/features/admin/promotions/option_value_rule_spec.rb:18 # Promotion with option value rule adding an option value rule
rspec ./spec/features/admin/promotions/option_value_rule_spec.rb:78 # Promotion with option value rule with an existing option value rule deleting a product

Randomized with seed 15122
```
@gsmendoza
Avoid persisting Spree::Role objects as UserRole role IDs
11b37c9 
Prevents spec for raising `Psych::DisallowedClass: Tried to load
unspecified class: Spree::Role` errors.

See https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017.
@gsmendoza gsmendoza marked this pull request as ready for review July 15, 2022 00:21
@gsmendoza gsmendoza force-pushed the gsmendoza/eng-401-update-solidus-31-to-support-cve-2022 branch from 8cd8362 to 862b3fb Compare July 15, 2022 00:25
@gsmendoza gsmendoza changed the title Backport to v3.1: Support CVE-2022-32224 Rails security updates Support CVE-2022-32224 Rails security updates - Backport to v3.1 Jul 15, 2022
@gsmendoza gsmendoza changed the title Support CVE-2022-32224 Rails security updates - Backport to v3.1 Support CVE-2022-32224 Rails security updates - backport to v3.1 Jul 15, 2022
kennyadsl
kennyadsl approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsmendoza appoved! But can you please link the backported PRs in the description?

@gsmendoza gsmendoza mentioned this pull request Jul 15, 2022
Merged
2 tasks
@gsmendoza
Copy link
Contributor Author

Thanks @kennyadsl . I added a Backports section in the PR description of #4451.

This was referenced Jul 15, 2022
Merged
Merged
@waiting-for-dev waiting-for-dev merged commit 03bebb5 into solidusio:v3.1 Jul 15, 2022
@gsmendoza gsmendoza mentioned this pull request Jul 19, 2022
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennyadsl kennyadsl kennyadsl approved these changes

@waiting-for-dev waiting-for-dev waiting-for-dev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gsmendoza @waiting-for-dev @kennyadsl