

Denial of Service and Unsafe Object Creation Vulnerability in JSON Gem - ontoillogical
https://groups.google.com/forum/?fromgroups=#!topic/rubyonrails-security/4_YvCpLzL58

======
mapgrep
>`JSON.load` should _never_ be given input from unknown sources. If you are
processing JSON from an unknown source, _always_ use `JSON.parse`.

This seems like poor method naming; I would not intuitively understand that
"load" is far more dangerous than "parse."

Why not deprecate these and do names like

JSON.load_trusted

JSON.load_untrusted

~~~
dbaupp
Better would be load and load_trusted so that the safer function has the short
(and expected) name.

~~~
MatthewPhillips
Why have the unsafe function at all?

------
Tho85
Some details on how this can be exploited:

[http://www.zweitag.de/en/blog/ruby-on-rails-vulnerable-to-
ma...](http://www.zweitag.de/en/blog/ruby-on-rails-vulnerable-to-mass-
assignment-and-sql-injection)

~~~
tenderlove
Thanks for reporting this issue to us! :-D

<3<3<3<3

~~~
Tho85
Was a pleasure!

With love :-) Thomas

------
lkrubner
I apologize for the ignorant question, but how does Ruby survive this in
normal operation?

"Since Ruby symbols are not garbage collected, this can result in a denial of
service attack."

If you have a long running Ruby app,and it does not garbage collect symbols,
then those symbols are... constants I guess?That survive till the app stops
operating? So I guess the assumption is that no app should use too many
symbols (and they don't use much memory anyway?)

~~~
oinksoft
This is not an ignorant question. Any half-experienced Erlang developer can
tell you that you use `list_to_existing_atom' rather than `list_to_atom' if
you are ever doing dynamic things with atoms. So, if you're trying to
accomplish dynamic module lookup and foo_baz.beam is in your code path,
default startup will create the `foo_baz' atom, and you know that this atom
will exist at runtime.

Symbols in Ruby are atoms (the term "atom" spans languages), and GC/space
issues plague any persistent term like an atom, in any language.

And thus I arrive at a key question: Does Ruby have something like
`list_to_existing_atom', or some mechanism for telling if a symbol exists
already? I see no analog to this, only the `ID2SYM' macro in the extensions
API, and similar calls like String#to_sym.

Perhaps there is some way to clean up symbols after they are created. This to
me would seem like the ideal route. It's good they've got a stop-gap fix by
changing defaults, but it feels to me like they're punting here. Perhaps users
who do [ab]use this feature also would not like DOS attacks?

I hope others who know more about Ruby extension development, and symbol
management capabilities, can chime in on these questions.

~~~
benmmurphy
there are methods in the ruby reflection api that take strings and silently
convert them to symbols so it is really easy to accidentally leak memory.

however, on ruby trunk they added a method: rb_check_id which can be used to
check if a string has been already symbolized
(<https://github.com/ruby/ruby/blob/trunk/parse.y#L10465>). this means when
these reflection methods get passed a string and it hasn't been symbolized
they can bail out and not symbolize the string.
(<https://github.com/ruby/ruby/blob/trunk/object.c#L2073>)

~~~
oinksoft
This is an excellent development.

Indeed, when researching just now, I saw examples of people throwing strings
at Module#const_defined?, which no doubt get converted to symbols
straightaway.

------
benmmurphy
also if you have done require 'json/add/rails' you are in for fun
([https://github.com/ruby/ruby/blob/v1_9_2_381/ext/json/lib/js...](https://github.com/ruby/ruby/blob/v1_9_2_381/ext/json/lib/json/add/rails.rb#L10))

    
    
        irb(main):001:0> require 'json/add/rails'
        => true
        irb(main):002:0> class Foo
        irb(main):003:1> end
        => nil
        irb(main):004:0> Foo.json_create({"x" => "bar"})
        => #<Foo:0x007fc5f3149540>
    

[https://github.com/search?q=require+%27json%2Fadd%2Frails%27...](https://github.com/search?q=require+%27json%2Fadd%2Frails%27&type=Code&ref=searchresults)

------
zyang
Is it Monday again?

