Skip to content
Permalink
Browse files

Use Psych.safe_load on gem pusher

Psych.safe_load is what we want to use for rubygems, this YAML monkey
patch was probably done before that existed, however it can be way
simpler now as Psych provides a safe load method.
  • Loading branch information...
arthurnn committed Feb 6, 2015
1 parent 24a2e48 commit 63bb533ffc3543f4aab049d3af4f83b606442044
Showing with 22 additions and 78 deletions.
  1. +1 −1 app/models/pusher.rb
  2. +20 −75 config/initializers/forbidden_yaml.rb
  3. +1 −2 test/unit/pusher_test.rb
@@ -53,7 +53,7 @@ def pull_spec

raise Gem::Package::FormatError.new('package metadata is missing') unless @spec
@spec
rescue Psych::WhitelistException => e
rescue Psych::Exception => e
Rails.logger.info "Attempted YAML metadata exploit: #{e}"
notify("RubyGems.org cannot process this gem.\nThe metadata is invalid.\n#{e}", 422)
rescue Gem::Package::FormatError
@@ -6,79 +6,28 @@
# Assert we're using Psych
abort "Use Psych for YAML, install libyaml and reinstall ruby" unless YAML == Psych

module Psych
class WhitelistException < Exception; end
class ForbiddenClassException < WhitelistException; end
class ForbiddenSymbolException < WhitelistException; end

WHITELISTED_CLASSES = %w(
Gem::Dependency
Gem::Platform
Gem::Requirement
Gem::Specification
Gem::Version
Gem::Version::Requirement
)

# These are all unique symbols used across all currently published gems' metadata
WHITELISTED_SYMBOLS = %w(
development
runtime
)

class WhitelistedScalarScanner < ScalarScanner
def tokenize string
# Protect against scanned scalars which will become symbols
if string[/^:./]
symbol = string.sub(/^:/, "")
symbol = $2 if string =~ /^(["'])(.*)\1/

raise ForbiddenSymbolException, "Forbidden symbol in YAML: #{symbol}" unless WHITELISTED_SYMBOLS.include? symbol
end

super
end
end

module Visitors
class WhitelistedToRuby < ToRuby
def initialize
super
@ss = WhitelistedScalarScanner.new
end

def visit_Psych_Nodes_Scalar o
# Protect against explicitly tagged ruby classes which take the YAML shortcut, i.e. ActiveRecord::Base
if klass = Psych.load_tags[o.tag]
raise ForbiddenClassException, "Forbidden class in YAML: #{klassname}" unless WHITELISTED_CLASSES.include? klass.name
end

# Protect against explicitly tagged ruby symbols
if o.tag and o.tag[/^!ruby\/sym(bol)?:?(.*)?$/]
raise ForbiddenSymbolException, "Forbidden symbol in YAML: #{o.value}" unless WHITELISTED_SYMBOLS.include? o.value
end

super
end

private

def resolve_class klassname
# Protect against all explicit ruby classes, from tags or otherwise
raise ForbiddenClassException, "Forbidden class in YAML: #{klassname}" unless WHITELISTED_CLASSES.include? klassname

super klassname
end
end
end
end

module Gem
class Specification
WHITELISTED_CLASSES = %w(
Symbol
Time
Date
Gem::Dependency
Gem::Platform
Gem::Requirement
Gem::Specification
Gem::Version
Gem::Version::Requirement
)

WHITELISTED_SYMBOLS = %w(
development
runtime
)

def self.from_yaml input
input = normalize_yaml_input input
nodes = Psych.parse input
spec = Psych::Visitors::WhitelistedToRuby.new.accept nodes
spec = Psych.safe_load(input, WHITELISTED_CLASSES, WHITELISTED_SYMBOLS)

if spec && spec.class == FalseClass then
raise Gem::EndOfYAMLException
@@ -88,12 +37,8 @@ def self.from_yaml input
raise Gem::Exception, "YAML data doesn't evaluate to gem specification"
end

unless (spec.instance_variables.include? '@specification_version' or
spec.instance_variables.include? :@specification_version) and
spec.instance_variable_get :@specification_version
spec.instance_variable_set :@specification_version,
NONEXISTENT_SPECIFICATION_VERSION
end
spec.specification_version ||= NONEXISTENT_SPECIFICATION_VERSION
spec.reset_nil_attributes_to_default

spec
end
@@ -103,8 +103,7 @@ class PusherTest < ActiveSupport::TestCase
assert_nil @cutter.spec
assert_includes @cutter.message, %{RubyGems.org cannot process this gem}
assert_includes @cutter.message, %{The metadata is invalid}
assert_includes @cutter.message, %{Forbidden symbol in YAML}
assert_includes @cutter.message, %{badsymbol}
assert_includes @cutter.message, %{Tried to load unspecified class: Symbol}
assert_equal @cutter.code, 422
end
end

0 comments on commit 63bb533

Please sign in to comment.
You can’t perform that action at this time.