Created
February 11, 2013 19:15
-
-
Save bjhess/4756802 to your computer and use it in GitHub Desktop.
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
--- | |
CHANGES | 8 ++++++ | |
Gemfile | 1 + | |
ext/json/ext/parser/parser.c | 2 +- | |
ext/json/ext/parser/parser.rl | 2 +- | |
java/src/json/ext/Parser.java | 2 +- | |
java/src/json/ext/Parser.rl | 2 +- | |
json.gemspec | 2 +- | |
json_pure.gemspec | 2 +- | |
lib/json/common.rb | 21 +++++++++----- | |
lib/json/generic_object.rb | 7 +++++ | |
lib/json/pure/parser.rb | 8 +++--- | |
tests/test_json.rb | 10 +++++-- | |
tests/test_json_addition.rb | 56 ++++++++++++++++++++++---------------- | |
tests/test_json_generic_object.rb | 30 ++++++++++++++------ | |
tests/test_json_string_matching.rb | 7 ++--- | |
15 files changed, 105 insertions(+), 55 deletions(-) | |
diff --git a/CHANGES b/CHANGES | |
index a8c0b35..e3d12a7 100644 | |
--- a/CHANGES | |
+++ b/CHANGES | |
@@ -1,4 +1,12 @@ | |
2013-02-04 (1.7.7) | |
+ * Security fix for JSON create_additions default value and | |
+ JSON::GenericObject. It should not be possible to create additions unless | |
+ explicitely requested by setting the create_additions argument to true or | |
+ using the JSON.load/dump interface. If JSON::GenericObject is supposed to | |
+ be automatically deserialised, this has to be explicitely enabled by | |
+ setting | |
+ JSON::GenericObject.json_createble = true | |
+ as well. | |
* Remove useless assert in fbuffer implementation. | |
* Apply patch attached to https://github.com/flori/json/issues#issue/155 | |
provided by John Shahid <[email protected]>, Thx! | |
diff --git a/Gemfile b/Gemfile | |
index 98d7837..e405da2 100644 | |
--- a/Gemfile | |
+++ b/Gemfile | |
@@ -8,3 +8,4 @@ gemspec :name => 'json-java' | |
gem 'utils' | |
gem 'test-unit' | |
+gem 'debugger', :platform => :mri_19 | |
diff --git a/ext/json/ext/parser/parser.c b/ext/json/ext/parser/parser.c | |
index 8442d21..df89f2c 100644 | |
--- a/ext/json/ext/parser/parser.c | |
+++ b/ext/json/ext/parser/parser.c | |
@@ -1680,7 +1680,7 @@ static VALUE cParser_initialize(int argc, VALUE *argv, VALUE self) | |
if (option_given_p(opts, tmp)) { | |
json->create_additions = RTEST(rb_hash_aref(opts, tmp)); | |
} else { | |
- json->create_additions = 1; | |
+ json->create_additions = 0; | |
} | |
tmp = ID2SYM(i_create_id); | |
if (option_given_p(opts, tmp)) { | |
diff --git a/ext/json/ext/parser/parser.rl b/ext/json/ext/parser/parser.rl | |
index 6138a6f..ab8d318 100644 | |
--- a/ext/json/ext/parser/parser.rl | |
+++ b/ext/json/ext/parser/parser.rl | |
@@ -664,7 +664,7 @@ static VALUE cParser_initialize(int argc, VALUE *argv, VALUE self) | |
if (option_given_p(opts, tmp)) { | |
json->create_additions = RTEST(rb_hash_aref(opts, tmp)); | |
} else { | |
- json->create_additions = 1; | |
+ json->create_additions = 0; | |
} | |
tmp = ID2SYM(i_create_id); | |
if (option_given_p(opts, tmp)) { | |
diff --git a/java/src/json/ext/Parser.java b/java/src/json/ext/Parser.java | |
index ab3585e..6cb5886 100644 | |
--- a/java/src/json/ext/Parser.java | |
+++ b/java/src/json/ext/Parser.java | |
@@ -166,7 +166,7 @@ public class Parser extends RubyObject { | |
this.symbolizeNames = opts.getBool("symbolize_names", false); | |
this.quirksMode = opts.getBool("quirks_mode", false); | |
this.createId = opts.getString("create_id", getCreateId(context)); | |
- this.createAdditions = opts.getBool("create_additions", true); | |
+ this.createAdditions = opts.getBool("create_additions", false); | |
this.objectClass = opts.getClass("object_class", runtime.getHash()); | |
this.arrayClass = opts.getClass("array_class", runtime.getArray()); | |
this.match_string = opts.getHash("match_string"); | |
diff --git a/java/src/json/ext/Parser.rl b/java/src/json/ext/Parser.rl | |
index e26637d..6dd335a 100644 | |
--- a/java/src/json/ext/Parser.rl | |
+++ b/java/src/json/ext/Parser.rl | |
@@ -164,7 +164,7 @@ public class Parser extends RubyObject { | |
this.symbolizeNames = opts.getBool("symbolize_names", false); | |
this.quirksMode = opts.getBool("quirks_mode", false); | |
this.createId = opts.getString("create_id", getCreateId(context)); | |
- this.createAdditions = opts.getBool("create_additions", true); | |
+ this.createAdditions = opts.getBool("create_additions", false); | |
this.objectClass = opts.getClass("object_class", runtime.getHash()); | |
this.arrayClass = opts.getClass("array_class", runtime.getArray()); | |
this.match_string = opts.getHash("match_string"); | |
diff --git a/json.gemspec b/json.gemspec | |
index fb52be8..8d7c693 100644 | |
--- a/json.gemspec | |
+++ b/json.gemspec | |
@@ -6,7 +6,7 @@ Gem::Specification.new do |s| | |
s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= | |
s.authors = ["Florian Frank"] | |
- s.date = "2013-02-04" | |
+ s.date = "2013-02-10" | |
s.description = "This is a JSON implementation as a Ruby extension in C." | |
s.email = "[email protected]" | |
s.extensions = ["ext/json/ext/generator/extconf.rb", "ext/json/ext/parser/extconf.rb"] | |
diff --git a/json_pure.gemspec b/json_pure.gemspec | |
index 1d4b4c0..0d696c9 100644 | |
--- a/json_pure.gemspec | |
+++ b/json_pure.gemspec | |
@@ -6,7 +6,7 @@ Gem::Specification.new do |s| | |
s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= | |
s.authors = ["Florian Frank"] | |
- s.date = "2013-02-04" | |
+ s.date = "2013-02-10" | |
s.description = "This is a JSON implementation in pure Ruby." | |
s.email = "[email protected]" | |
s.extra_rdoc_files = ["README.rdoc"] | |
diff --git a/lib/json/common.rb b/lib/json/common.rb | |
index 03892d9..65a74a1 100644 | |
--- a/lib/json/common.rb | |
+++ b/lib/json/common.rb | |
@@ -299,21 +299,28 @@ module JSON | |
attr_accessor :load_default_options | |
end | |
self.load_default_options = { | |
- :max_nesting => false, | |
- :allow_nan => true, | |
- :quirks_mode => true, | |
+ :max_nesting => false, | |
+ :allow_nan => true, | |
+ :quirks_mode => true, | |
+ :create_additions => true, | |
} | |
# Load a ruby data structure from a JSON _source_ and return it. A source can | |
# either be a string-like object, an IO-like object, or an object responding | |
# to the read method. If _proc_ was given, it will be called with any nested | |
- # Ruby object as an argument recursively in depth first order. The default | |
- # options for the parser can be changed via the load_default_options method. | |
+ # Ruby object as an argument recursively in depth first order. To modify the | |
+ # default options pass in the optional _options_ argument as well. | |
+ # | |
+ # BEWARE: This method is meant to serialise data from trusted user input, | |
+ # like from your own database server or clients under your control, it could | |
+ # be dangerous to allow untrusted users to pass JSON sources into it. The | |
+ # default options for the parser can be changed via the load_default_options | |
+ # method. | |
# | |
# This method is part of the implementation of the load/dump interface of | |
# Marshal and YAML. | |
- def load(source, proc = nil) | |
- opts = load_default_options | |
+ def load(source, proc = nil, options = {}) | |
+ opts = load_default_options.merge options | |
if source.respond_to? :to_str | |
source = source.to_str | |
elsif source.respond_to? :to_io | |
diff --git a/lib/json/generic_object.rb b/lib/json/generic_object.rb | |
index cd93e1a..8b1074c 100644 | |
--- a/lib/json/generic_object.rb | |
+++ b/lib/json/generic_object.rb | |
@@ -5,6 +5,12 @@ module JSON | |
class << self | |
alias [] new | |
+ def json_creatable? | |
+ @json_creatable | |
+ end | |
+ | |
+ attr_writer :json_creatable | |
+ | |
def json_create(data) | |
data = data.dup | |
data.delete JSON.create_id | |
@@ -26,6 +32,7 @@ module JSON | |
end | |
end | |
end | |
+ self.json_creatable = false | |
def to_hash | |
table | |
diff --git a/lib/json/pure/parser.rb b/lib/json/pure/parser.rb | |
index cb249b2..a41d1ee 100644 | |
--- a/lib/json/pure/parser.rb | |
+++ b/lib/json/pure/parser.rb | |
@@ -63,9 +63,9 @@ module JSON | |
# * *symbolize_names*: If set to true, returns symbols for the names | |
# (keys) in a JSON object. Otherwise strings are returned, which is also | |
# the default. | |
- # * *create_additions*: If set to false, the Parser doesn't create | |
- # additions even if a matchin class and create_id was found. This option | |
- # defaults to true. | |
+ # * *create_additions*: If set to true, the Parser creates | |
+ # additions when if a matching class and create_id was found. This | |
+ # option defaults to false. | |
# * *object_class*: Defaults to Hash | |
# * *array_class*: Defaults to Array | |
# * *quirks_mode*: Enables quirks_mode for parser, that is for example | |
@@ -88,7 +88,7 @@ module JSON | |
if opts.key?(:create_additions) | |
@create_additions = !!opts[:create_additions] | |
else | |
- @create_additions = true | |
+ @create_additions = false | |
end | |
@create_id = @create_additions ? JSON.create_id : nil | |
@object_class = opts[:object_class] || Hash | |
diff --git a/tests/test_json.rb b/tests/test_json.rb | |
index be974cd..6af6b32 100755 | |
--- a/tests/test_json.rb | |
+++ b/tests/test_json.rb | |
@@ -329,12 +329,12 @@ class TestJSON < Test::Unit::TestCase | |
def test_generate_core_subclasses_with_new_to_json | |
obj = SubHash2["foo" => SubHash2["bar" => true]] | |
obj_json = JSON(obj) | |
- obj_again = JSON(obj_json) | |
+ obj_again = JSON.parse(obj_json, :create_additions => true) | |
assert_kind_of SubHash2, obj_again | |
assert_kind_of SubHash2, obj_again['foo'] | |
assert obj_again['foo']['bar'] | |
assert_equal obj, obj_again | |
- assert_equal ["foo"], JSON(JSON(SubArray2["foo"])) | |
+ assert_equal ["foo"], JSON(JSON(SubArray2["foo"]), :create_additions => true) | |
end | |
def test_generate_core_subclasses_with_default_to_json | |
@@ -493,6 +493,12 @@ EOT | |
assert_equal nil, JSON.load('') | |
end | |
+ def test_load_with_options | |
+ small_hash = JSON("foo" => 'bar') | |
+ symbol_hash = { :foo => 'bar' } | |
+ assert_equal symbol_hash, JSON.load(small_hash, nil, :symbolize_names => true) | |
+ end | |
+ | |
def test_dump | |
too_deep = '[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]' | |
assert_equal too_deep, JSON.dump(eval(too_deep)) | |
diff --git a/tests/test_json_addition.rb b/tests/test_json_addition.rb | |
index 707aa32..a30f06a 100755 | |
--- a/tests/test_json_addition.rb | |
+++ b/tests/test_json_addition.rb | |
@@ -73,11 +73,19 @@ class TestJSONAddition < Test::Unit::TestCase | |
a = A.new(666) | |
assert A.json_creatable? | |
json = generate(a) | |
- a_again = JSON.parse(json) | |
+ a_again = JSON.parse(json, :create_additions => true) | |
assert_kind_of a.class, a_again | |
assert_equal a, a_again | |
end | |
+ def test_extended_json_default | |
+ a = A.new(666) | |
+ assert A.json_creatable? | |
+ json = generate(a) | |
+ a_hash = JSON.parse(json) | |
+ assert_kind_of Hash, a_hash | |
+ end | |
+ | |
def test_extended_json_disabled | |
a = A.new(666) | |
assert A.json_creatable? | |
@@ -104,7 +112,7 @@ class TestJSONAddition < Test::Unit::TestCase | |
c = C.new | |
assert !C.json_creatable? | |
json = generate(c) | |
- assert_raises(ArgumentError, NameError) { JSON.parse(json) } | |
+ assert_raises(ArgumentError, NameError) { JSON.parse(json, :create_additions => true) } | |
end | |
def test_raw_strings | |
@@ -122,7 +130,7 @@ class TestJSONAddition < Test::Unit::TestCase | |
assert_match(/\A\{.*\}\z/, json) | |
assert_match(/"json_class":"String"/, json) | |
assert_match(/"raw":\[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255\]/, json) | |
- raw_again = JSON.parse(json) | |
+ raw_again = JSON.parse(json, :create_additions => true) | |
assert_equal raw, raw_again | |
end | |
@@ -130,17 +138,17 @@ class TestJSONAddition < Test::Unit::TestCase | |
def test_core | |
t = Time.now | |
- assert_equal t, JSON(JSON(t)) | |
+ assert_equal t, JSON(JSON(t), :create_additions => true) | |
d = Date.today | |
- assert_equal d, JSON(JSON(d)) | |
+ assert_equal d, JSON(JSON(d), :create_additions => true) | |
d = DateTime.civil(2007, 6, 14, 14, 57, 10, Rational(1, 12), 2299161) | |
- assert_equal d, JSON(JSON(d)) | |
- assert_equal 1..10, JSON(JSON(1..10)) | |
- assert_equal 1...10, JSON(JSON(1...10)) | |
- assert_equal "a".."c", JSON(JSON("a".."c")) | |
- assert_equal "a"..."c", JSON(JSON("a"..."c")) | |
+ assert_equal d, JSON(JSON(d), :create_additions => true) | |
+ assert_equal 1..10, JSON(JSON(1..10), :create_additions => true) | |
+ assert_equal 1...10, JSON(JSON(1...10), :create_additions => true) | |
+ assert_equal "a".."c", JSON(JSON("a".."c"), :create_additions => true) | |
+ assert_equal "a"..."c", JSON(JSON("a"..."c"), :create_additions => true) | |
s = MyJsonStruct.new 4711, 'foot' | |
- assert_equal s, JSON(JSON(s)) | |
+ assert_equal s, JSON(JSON(s), :create_additions => true) | |
struct = Struct.new :foo, :bar | |
s = struct.new 4711, 'foot' | |
assert_raises(JSONError) { JSON(s) } | |
@@ -148,41 +156,41 @@ class TestJSONAddition < Test::Unit::TestCase | |
raise TypeError, "test me" | |
rescue TypeError => e | |
e_json = JSON.generate e | |
- e_again = JSON e_json | |
+ e_again = JSON e_json, :create_additions => true | |
assert_kind_of TypeError, e_again | |
assert_equal e.message, e_again.message | |
assert_equal e.backtrace, e_again.backtrace | |
end | |
- assert_equal(/foo/, JSON(JSON(/foo/))) | |
- assert_equal(/foo/i, JSON(JSON(/foo/i))) | |
+ assert_equal(/foo/, JSON(JSON(/foo/), :create_additions => true)) | |
+ assert_equal(/foo/i, JSON(JSON(/foo/i), :create_additions => true)) | |
end | |
def test_utc_datetime | |
now = Time.now | |
- d = DateTime.parse(now.to_s) # usual case | |
- assert_equal d, JSON.parse(d.to_json) | |
+ d = DateTime.parse(now.to_s, :create_additions => true) # usual case | |
+ assert_equal d, JSON.parse(d.to_json, :create_additions => true) | |
d = DateTime.parse(now.utc.to_s) # of = 0 | |
- assert_equal d, JSON.parse(d.to_json) | |
+ assert_equal d, JSON.parse(d.to_json, :create_additions => true) | |
d = DateTime.civil(2008, 6, 17, 11, 48, 32, Rational(1,24)) | |
- assert_equal d, JSON.parse(d.to_json) | |
+ assert_equal d, JSON.parse(d.to_json, :create_additions => true) | |
d = DateTime.civil(2008, 6, 17, 11, 48, 32, Rational(12,24)) | |
- assert_equal d, JSON.parse(d.to_json) | |
+ assert_equal d, JSON.parse(d.to_json, :create_additions => true) | |
end | |
def test_rational_complex | |
- assert_equal Rational(2, 9), JSON(JSON(Rational(2, 9))) | |
- assert_equal Complex(2, 9), JSON(JSON(Complex(2, 9))) | |
+ assert_equal Rational(2, 9), JSON.parse(JSON(Rational(2, 9)), :create_additions => true) | |
+ assert_equal Complex(2, 9), JSON.parse(JSON(Complex(2, 9)), :create_additions => true) | |
end | |
def test_bigdecimal | |
- assert_equal BigDecimal('3.141', 23), JSON(JSON(BigDecimal('3.141', 23))) | |
- assert_equal BigDecimal('3.141', 666), JSON(JSON(BigDecimal('3.141', 666))) | |
+ assert_equal BigDecimal('3.141', 23), JSON(JSON(BigDecimal('3.141', 23)), :create_additions => true) | |
+ assert_equal BigDecimal('3.141', 666), JSON(JSON(BigDecimal('3.141', 666)), :create_additions => true) | |
end | |
def test_ostruct | |
o = OpenStruct.new | |
# XXX this won't work; o.foo = { :bar => true } | |
o.foo = { 'bar' => true } | |
- assert_equal o, JSON(JSON(o)) | |
+ assert_equal o, JSON.parse(JSON(o), :create_additions => true) | |
end | |
end | |
diff --git a/tests/test_json_generic_object.rb b/tests/test_json_generic_object.rb | |
index 83093b8..77ef22e 100644 | |
--- a/tests/test_json_generic_object.rb | |
+++ b/tests/test_json_generic_object.rb | |
@@ -20,17 +20,22 @@ class TestJSONGenericObject < Test::Unit::TestCase | |
end | |
def test_generate_json | |
- assert_equal @go, JSON(JSON(@go)) | |
+ switch_json_creatable do | |
+ assert_equal @go, JSON(JSON(@go), :create_additions => true) | |
+ end | |
end | |
def test_parse_json | |
- assert_equal @go, l = JSON('{ "json_class": "JSON::GenericObject", "a": 1, "b": 2 }') | |
- assert_equal 1, l.a | |
- assert_equal @go, l = JSON('{ "a": 1, "b": 2 }', :object_class => GenericObject) | |
- assert_equal 1, l.a | |
- assert_equal GenericObject[:a => GenericObject[:b => 2]], | |
- l = JSON('{ "a": { "b": 2 } }', :object_class => GenericObject) | |
- assert_equal 2, l.a.b | |
+ assert_kind_of Hash, JSON('{ "json_class": "JSON::GenericObject", "a": 1, "b": 2 }', :create_additions => true) | |
+ switch_json_creatable do | |
+ assert_equal @go, l = JSON('{ "json_class": "JSON::GenericObject", "a": 1, "b": 2 }', :create_additions => true) | |
+ assert_equal 1, l.a | |
+ assert_equal @go, l = JSON('{ "a": 1, "b": 2 }', :object_class => GenericObject) | |
+ assert_equal 1, l.a | |
+ assert_equal GenericObject[:a => GenericObject[:b => 2]], | |
+ l = JSON('{ "a": { "b": 2 } }', :object_class => GenericObject) | |
+ assert_equal 2, l.a.b | |
+ end | |
end | |
def test_from_hash | |
@@ -43,4 +48,13 @@ class TestJSONGenericObject < Test::Unit::TestCase | |
assert_equal true, result.foo.quux.first.foobar | |
assert_equal true, GenericObject.from_hash(true) | |
end | |
+ | |
+ private | |
+ | |
+ def switch_json_creatable | |
+ JSON::GenericObject.json_creatable = true | |
+ yield | |
+ ensure | |
+ JSON::GenericObject.json_creatable = false | |
+ end | |
end | |
diff --git a/tests/test_json_string_matching.rb b/tests/test_json_string_matching.rb | |
index 2ddedfa..c233df8 100644 | |
--- a/tests/test_json_string_matching.rb | |
+++ b/tests/test_json_string_matching.rb | |
@@ -27,14 +27,13 @@ class TestJSONStringMatching < Test::Unit::TestCase | |
t = TestTime.new | |
t_json = [ t ].to_json | |
assert_equal [ t ], | |
- JSON.parse(t_json, | |
+ JSON.parse(t_json, :create_additions => true, | |
:match_string => { /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{4}\z/ => TestTime }) | |
assert_equal [ t.strftime('%FT%T%z') ], | |
- JSON.parse(t_json, | |
+ JSON.parse(t_json, :create_additions => true, | |
:match_string => { /\A\d{3}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{4}\z/ => TestTime }) | |
assert_equal [ t.strftime('%FT%T%z') ], | |
JSON.parse(t_json, | |
- :match_string => { /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{4}\z/ => TestTime }, | |
- :create_additions => false) | |
+ :match_string => { /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{4}\z/ => TestTime }) | |
end | |
end | |
-- | |
1.8.1.2 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment