search

philnash / bitly

🗜 A Ruby wrapper for the bit.ly API
https://rubygems.org/gems/bitly
MIT License
451 stars 139 forks source link

Replace crack with multi_json #34

Closed urbanautomaton closed 11 years ago

urbanautomaton commented 11 years ago

The crack gem uses an extracted copy of an outdated Rails JSON parser, which uses YAML as a backend. This is vulnerable to arbitrary code execution in certain contexts:

https://groups.google.com/forum/#!topic/rubyonrails-security/1h2DR63ViGo

While I admit it's highly unlikely that bit.ly API responses will contain malicious input, I would very much like to be able to use the bitly gem without needing to install an unsafe dependency. This patch replaces crack with multi_json, which intelligently selects from the locally available JSON backends, placing minimal constraints on client dependencies.

I wasn't sure if updating the gemspec was the right thing to do - I figured it was best on grounds of consistency. Apologies if not; I'm happy to amend as necessary.

Cheers (and thanks for providing this software!), Simon

philnash commented 11 years ago

Wow, sorry for the slow reply, I'm trying to catch up on issues right now.

Weird thing with this, the library seems to run ok, but a bunch of tests fail with NoMethodError: private method 'load' called for MultiJson:Module. Are you getting this, or is there something up with my environment?

urbanautomaton commented 11 years ago

Hi - no worries, this is far from a high priority, I'm sure. :) Anyway - no, I don't get any problems, and that's with a completely clean gemset (I see deprecation messages for the bitly v2 API in half the tests, but I presume that's normal). I'm running with MultiJson 1.5.0 - what version do you have?

Bit of a bizarre error - MultiJson.load certainly doesn't look private in the source - a bit of searching suggests others have had the same error, though, without throwing up any solutions. :-\

philnash commented 11 years ago

Don't know what's going on with my setup, but my tests are including version 1.0.3 of MultiJson whereas just loading the library gets a later version that actually does have the load method. I'll clear out my environment, check the tests again, but this should be merged in soon!

philnash commented 11 years ago

I merged #33 before this and it sorted all my issues out (should have sorted those development dependencies before). I updated the requirement on multi_json to ~> 1.3 as 1.3 was the first version with the load/dump methods over decode/encode.

urbanautomaton commented 11 years ago

Ah, nice one - thanks for merging!