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

Use yaml_unsafe_load to handle symbols #21988

Merged
merged 1 commit into from
Jul 12, 2022
Merged

Use yaml_unsafe_load to handle symbols #21988

merged 1 commit into from
Jul 12, 2022

Conversation

agrare
Copy link
Member

@agrare agrare commented Jul 12, 2022 •
edited
Loading

Prevent Psych::DisallowedClass: Tried to load unspecified class: Symbol while seeding due to the default change during the rails 6.0.5.1 release.

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

NOTE the changelog has config.active_storage.use_yaml_unsafe_load but the actual config option is config.active_record.use_yaml_unsafe_load

$ rails db:seed
** ManageIQ master, codename: Oparin
rails aborted!
Psych::DisallowedClass: Tried to load unspecified class: Symbol
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/coders/yaml_column.rb:50:in `yaml_load'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/coders/yaml_column.rb:26:in `load'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/type/serialized.rb:22:in `deserialize'
/home/grare/adam/.gem/ruby/2.7.0/gems/activemodel-6.0.5.1/lib/active_model/type/helpers/mutable.rb:8:in `cast'
/home/grare/adam/.gem/ruby/2.7.0/gems/activemodel-6.0.5.1/lib/active_model/attribute.rb:174:in `type_cast'
/home/grare/adam/.gem/ruby/2.7.0/gems/activemodel-6.0.5.1/lib/active_model/attribute.rb:42:in `value'
/home/grare/adam/.gem/ruby/2.7.0/gems/activemodel-6.0.5.1/lib/active_model/attribute_set.rb:28:in `transform_values'
/home/grare/adam/.gem/ruby/2.7.0/gems/activemodel-6.0.5.1/lib/active_model/attribute_set.rb:28:in `to_hash'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/attribute_methods.rb:261:in `attributes'
/home/grare/adam/.gem/ruby/2.7.0/gems/default_value_for-3.4.0/lib/default_value_for.rb:160:in `block in set_default_values'
/home/grare/adam/.gem/ruby/2.7.0/gems/default_value_for-3.4.0/lib/default_value_for.rb:155:in `each'
/home/grare/adam/.gem/ruby/2.7.0/gems/default_value_for-3.4.0/lib/default_value_for.rb:155:in `set_default_values'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:428:in `block in make_lambda'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:238:in `block in halting_and_conditional'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:517:in `block in invoke_after'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:517:in `each'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:517:in `invoke_after'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:136:in `run_callbacks'
/home/grare/adam/.gem/ruby/2.7.0/gems/activesupport-6.0.5.1/lib/active_support/callbacks.rb:825:in `_run_initialize_callbacks'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/core.rb:337:in `initialize'
/home/grare/adam/.gem/ruby/2.7.0/gems/default_value_for-3.4.0/lib/default_value_for.rb:143:in `initialize'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/inheritance.rb:70:in `new'
/home/grare/adam/.gem/ruby/2.7.0/gems/activerecord-6.0.5.1/lib/active_record/inheritance.rb:70:in `new'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_user_role.rb:93:in `block in seed_from_array'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_user_role.rb:90:in `each'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_user_role.rb:90:in `seed_from_array'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_user_role.rb:80:in `seed'

@agrare agrare requested a review from jrafanie as a code owner July 12, 2022 19:29
@agrare
Use yaml_unsafe_load to handle symbols
d339213 
Prevent `Psych::DisallowedClass: Tried to load unspecified class: Symbol`
while seeding due to the default change during the rails 6.0.5.1
release.
@agrare agrare force-pushed the use_yaml_unsafe_load branch from 1dc36d0 to d339213 Compare July 12, 2022 19:29
@agrare agrare requested a review from Fryguy as a code owner July 12, 2022 19:29
jrafanie
jrafanie approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

LGTM. I can't believe this was changed in 6.0.5.1. 😭

@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2022

Checked commit agrare@d339213 with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@kbrock kbrock self-assigned this Jul 12, 2022
@kbrock kbrock merged commit a4f4636 into ManageIQ:master Jul 12, 2022
@agrare agrare deleted the use_yaml_unsafe_load branch July 12, 2022 20:05
@Fryguy
Copy link
Member

Fryguy commented Jul 12, 2022

Will this have to go back to najdorf for security reasons?

@agrare
Copy link
Member Author

agrare commented Jul 12, 2022

The release was for a CVE so yes

@Fryguy
Copy link
Member

Fryguy commented Jul 13, 2022

Backported to najdorf in commit 705cdb7.

commit 705cdb74144974ad7427e5f79a197b5294841195
Author: Keenan Brock <[email protected]>
Date:   Tue Jul 12 16:05:16 2022 -0400

    Merge pull request #21988 from agrare/use_yaml_unsafe_load
    
    Use yaml_unsafe_load to handle symbols
    
    (cherry picked from commit a4f46361e660ff468399ce86446ea1de4e8ddcc9)

Fryguy pushed a commit that referenced this pull request Jul 13, 2022
@kbrock @Fryguy
Merge pull request #21988 from agrare/use_yaml_unsafe_load
705cdb7 
Use yaml_unsafe_load to handle symbols

(cherry picked from commit a4f4636)
@Fryguy Fryguy mentioned this pull request Jul 13, 2022
Merged
jrafanie added a commit to jrafanie/manageiq-schema that referenced this pull request Jul 13, 2022
@jrafanie
Update the dummy app to use_yaml_unsafe_load in rails 6.0.5.1
6b7d6fd 
Rails 6.0.5.1 changed the default behavior to use yaml safe load in 6.0.5.1 so
we need to tell it use the old behavior in the dummy app.

Related to the core change in:
ManageIQ/manageiq#21988
@jrafanie jrafanie mentioned this pull request Jul 13, 2022
Merged
@Fryguy
Copy link
Member

Fryguy commented Sep 19, 2022

Backported to morphy in commit cc10a37.

commit cc10a37b02836b1d9fb110a190557866bb01ea85
Author: Keenan Brock <[email protected]>
Date:   Tue Jul 12 16:05:16 2022 -0400

    Merge pull request #21988 from agrare/use_yaml_unsafe_load
    
    Use yaml_unsafe_load to handle symbols
    
    (cherry picked from commit a4f46361e660ff468399ce86446ea1de4e8ddcc9)

Fryguy pushed a commit that referenced this pull request Sep 19, 2022
@kbrock @Fryguy
Merge pull request #21988 from agrare/use_yaml_unsafe_load
cc10a37 
Use yaml_unsafe_load to handle symbols

(cherry picked from commit a4f4636)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrafanie jrafanie jrafanie approved these changes

@Fryguy Fryguy Awaiting requested review from Fryguy Fryguy is a code owner

Assignees

@kbrock kbrock

Labels
bug dependencies morphy/backported najdorf/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@agrare @miq-bot @Fryguy @jrafanie @kbrock