Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Protect against symbol injection DoS attacks

  • Loading branch information...
commit d02c4c076a24c042021bec9f6883b96673652bcf 1 parent 8f2906e
@sj26 sj26 authored
View
2  app/models/pusher.rb
@@ -38,7 +38,7 @@ def pull_spec
end
false
- rescue Psych::ForbiddenClassException => e
+ rescue Psych::WhitelistException => 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
View
59 config/initializers/forbidden_yaml.rb
@@ -7,32 +7,69 @@
abort "Use Psych for YAML, install libyaml and reinstall ruby" unless YAML == Psych
module Psych
- class ForbiddenClassException < Exception
+ 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
+ json
+ mocha
+ rdoc
+ 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
- WHITELISTED_CLASSES = %w(
- Gem::Dependency
- Gem::Platform
- Gem::Requirement
- Gem::Specification
- Gem::Version
- Gem::Version::Requirement
- )
-
- def deserialize o
+ 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
View
BIN  test/gems/dos-1.0.0.gem
Binary file not shown
View
BIN  test/gems/dos-2.0.0.gem
Binary file not shown
View
BIN  test/gems/dos-3.0.0.gem
Binary file not shown
View
BIN  test/gems/dos-4.0.0.gem
Binary file not shown
View
14 test/unit/pusher_test.rb
@@ -91,6 +91,20 @@ class PusherTest < ActiveSupport::TestCase
assert_equal @cutter.code, 422
end
+ should "not be able to pull spec with metadata containing bad ruby symbols" do
+ ["1.0.0", "2.0.0", "3.0.0", "4.0.0"].each do |version|
+ @gem = gem_file("dos-#{version}.gem")
+ @cutter = Pusher.new(@user, @gem)
+ @cutter.pull_spec
+ assert_nil @cutter.spec
+ assert_include @cutter.message, %{RubyGems.org cannot process this gem}
+ assert_include @cutter.message, %{The metadata is invalid}
+ assert_include @cutter.message, %{Forbidden symbol in YAML}
+ assert_include @cutter.message, %{badsymbol}
+ assert_equal @cutter.code, 422
+ end
+ end
+
should "post info to the remote bundler API" do
@cutter.pull_spec
Please sign in to comment.
Something went wrong with that request. Please try again.