From 510b1638ac9bba3ceb7a5d73135dafff9e5bab49 Mon Sep 17 00:00:00 2001 From: Aaron Patterson <aaron.patterson@gmail.com> Date: Fri, 6 Oct 2017 11:11:40 -0700 Subject: [PATCH] Whitelist classes and symbols that are in Gem spec YAML This patch adds a method for loading YAML specs from a gem and whitelists classes and symbols that are allowed in the spec. Then it changes calls to YAML.load to call the whitelisted "safe" loader instead. [CVE-2017-0903] --- Manifest.txt | 1 + lib/rubygems.rb | 3 ++- lib/rubygems/config_file.rb | 2 +- lib/rubygems/package.rb | 2 +- lib/rubygems/package/old.rb | 2 +- lib/rubygems/safe_yaml.rb | 48 +++++++++++++++++++++++++++++++++++++++++++ lib/rubygems/specification.rb | 2 +- 7 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 lib/rubygems/safe_yaml.rb diff --git a/Manifest.txt b/Manifest.txt index 182ed4c130..c091e7c503 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -387,6 +387,7 @@ lib/rubygems/resolver/specification.rb lib/rubygems/resolver/stats.rb lib/rubygems/resolver/vendor_set.rb lib/rubygems/resolver/vendor_specification.rb +lib/rubygems/safe_yaml.rb lib/rubygems/security.rb lib/rubygems/security/policies.rb lib/rubygems/security/policy.rb diff --git a/lib/rubygems.rb b/lib/rubygems.rb index 55aa85b8b2..a13c6338cf 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -675,7 +675,7 @@ def self.load_yaml unless test_syck begin - gem 'psych', '>= 1.2.1' + gem 'psych', '>= 2.0.0' rescue Gem::LoadError # It's OK if the user does not have the psych gem installed. We will # attempt to require the stdlib version @@ -699,6 +699,7 @@ def self.load_yaml end require 'yaml' + require 'rubygems/safe_yaml' # If we're supposed to be using syck, then we may have to force # activate it via the YAML::ENGINE API. diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index c95d7dd1f1..63583b3616 100644 --- a/lib/rubygems/config_file.rb +++ b/lib/rubygems/config_file.rb @@ -345,7 +345,7 @@ def load_file(filename) return {} unless filename and File.exist? filename begin - content = YAML.load(File.read(filename)) + content = Gem::SafeYAML.load(File.read(filename)) unless content.kind_of? Hash warn "Failed to load #{filename} because it doesn't contain valid YAML hash" return {} diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index c36e71d800..77811ed5ec 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -468,7 +468,7 @@ def read_checksums gem @checksums = gem.seek 'checksums.yaml.gz' do |entry| Zlib::GzipReader.wrap entry do |gz_io| - YAML.load gz_io.read + Gem::SafeYAML.safe_load gz_io.read end end end diff --git a/lib/rubygems/package/old.rb b/lib/rubygems/package/old.rb index 5e722baa35..071f7141ab 100644 --- a/lib/rubygems/package/old.rb +++ b/lib/rubygems/package/old.rb @@ -101,7 +101,7 @@ def file_list io # :nodoc: header << line end - YAML.load header + Gem::SafeYAML.safe_load header end ## diff --git a/lib/rubygems/safe_yaml.rb b/lib/rubygems/safe_yaml.rb new file mode 100644 index 0000000000..b98cfaa5e6 --- /dev/null +++ b/lib/rubygems/safe_yaml.rb @@ -0,0 +1,48 @@ +module Gem + + ### + # This module is used for safely loading YAML specs from a gem. The + # `safe_load` method defined on this module is specifically designed for + # loading Gem specifications. For loading other YAML safely, please see + # Psych.safe_load + + module SafeYAML + WHITELISTED_CLASSES = %w( + Symbol + Time + Date + Gem::Dependency + Gem::Platform + Gem::Requirement + Gem::Specification + Gem::Version + Gem::Version::Requirement + YAML::Syck::DefaultKey + Syck::DefaultKey + ) + + WHITELISTED_SYMBOLS = %w( + development + runtime + ) + + if ::YAML.respond_to? :safe_load + def self.safe_load input + ::YAML.safe_load(input, WHITELISTED_CLASSES, WHITELISTED_SYMBOLS, true) + end + + def self.load input + ::YAML.safe_load(input, [::Symbol]) + end + else + warn "YAML safe loading is not available. Please upgrade psych to a version that supports safe loading (>= 2.0)." + def self.safe_load input, *args + ::YAML.load input + end + + def self.load input + ::YAML.load input + end + end + end +end diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 8218cd9d44..a9058baf10 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -1101,7 +1101,7 @@ def self.from_yaml(input) Gem.load_yaml input = normalize_yaml_input input - spec = YAML.load input + spec = Gem::SafeYAML.safe_load input if spec && spec.class == FalseClass then raise Gem::EndOfYAMLException