Harden safe load#799
Open
hsbt wants to merge 3 commits into
Open
Conversation
safe_load resolved !ruby/encoding directly via ::Encoding.find, bypassing the permitted_classes check that !ruby/object:Encoding already honors. Load it through the class loader so Encoding is only deserialized when permitted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
!ruby/hash-with-ivars, !ruby/hash and !map are only emitted for Hash subclasses, but the loader allocated whatever class the tag named and populated its ivars directly. That let a permitted non-Hash class be instantiated with attacker-chosen ivars, bypassing its init_with validation. Verify the resolved class is a Hash subclass before use. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
!ruby/array, !seq and !ruby/string carry the same exposure just fixed for the hash tags: the loader allocated the named class and replaced its contents without checking the class was actually an Array or String subclass. Apply the same subclass check so a permitted unrelated class can no longer be allocated and populated through these tags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens Psych’s YAML deserialization around several Ruby-specific tags to prevent safe/unsafe load bypasses (notably via choosing unrelated permitted classes and injecting ivars directly).
Changes:
- Route
!ruby/encodingthrough the class loader sopermitted_classesgoverns Encoding resolution. - Enforce that
!ruby/string,!ruby/array/!seq, and!ruby/hash*/!maptags only accept subclasses of their expected core types. - Add regression tests ensuring invalid (non-subclass) class names are rejected for these tags.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
lib/psych/visitors/to_ruby.rb |
Adds resolve_subclass and applies it to string/array/hash tag handling; routes !ruby/encoding through class loader. |
lib/psych/class_loader.rb |
Adds ENCODING constant so restricted loaders can gate Encoding via permitted_classes. |
test/psych/test_safe_load.rb |
Adds safe-load tests verifying !ruby/encoding is disallowed unless Encoding is permitted. |
test/psych/test_string.rb |
Adds tests ensuring !ruby/string:* rejects non-String classes. |
test/psych/test_array.rb |
Adds tests ensuring !seq:* / !ruby/array:* reject non-Array classes. |
test/psych/test_hash.rb |
Adds tests ensuring !ruby/hash*:* / !map:* reject non-Hash classes, covering ivar-injection bypass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+478
to
+484
| def resolve_subclass klassname, parent | ||
| klass = resolve_class(klassname) | ||
| if klass && !(klass <= parent) | ||
| raise ArgumentError, "Invalid tag: expected a subclass of #{parent}, got #{klass}" | ||
| end | ||
| klass | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
!ruby/encodingnow resolves through the class loader, sopermitted_classesis honored just like!ruby/object:Encoding.!ruby/hash-with-ivars,!ruby/hash,!mapnow require a Hash subclass, preventinginit_withbypass via direct ivar injection.!ruby/array,!seq,!ruby/stringnow require an Array/String subclass for the same reason.