Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Multiple vulnerabilities in parameter parsing in Action Pack

groups.google.com

Post reply on HN
Print view View original

261–270 of 294 posts

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#261

Post by FooBarWidget »

danso wrote: post #88
sdoowpilihp wrote: post #34

Can anyone with a more intimate knowledge of the inner workings of Ruby on Rails speak to how detrimental this exploit is in practice? I seem to recall a fair number of people feeling the SQL injection exploit from a few days ago was being blown out of proportion and I was wondering how this particular exploit stacks up against it.

I'm not going to say "told you so" because I said nothing and I'm just a layman in this...but when people were pointing out last week that the bug was "overblown" I had wondered if they were underestimating the tendency for such vulnerable patterns to propagate. The mechanisms that let even an edge case in are not always isolated.

Last week's bug is unrelated. Last week's bug was in ActiveRecord dynamic finders. This bug is in parameter and request input parsing.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#262

Post by FooBarWidget »

martinced wrote: post #239
danso wrote: post #88

Earlier quoted context omitted.

I'm not going to say "told you so" because I said nothing and I'm just a layman in this...but when people were pointing out last week that the bug was "overblown" I had wondered if they were underestimating the tendency for such vulnerable patterns to propagate. The mechanisms that let even an edge case in are not always isolated.

Oh I'm saying "told you so". Since years and years. The real problem is the very mentality of the people who downplay security issues, always saying "this is not a serious issue" (or, worse, saying "but language xxx / framework yyy" suffers from issues too, it's how the world works). That mentality is the reason why such exploits do exist in the first place. Security is nearly always an afterthought. The most brainde…

Or maybe some issues are overblown, while others are not.

Also, the message was not "overblown". It was "don't panic, but still upgrade ASAP".

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#263

Post by mattbee »

tptacek wrote: post #226
mattbee wrote: post #204

Earlier quoted context omitted.

Hmm, looking around I am thinking that if you don't want random object instantiation, this monkey-patch: module YAML; @@tagged_classes.delete('tag:ruby.yaml.org,2002:object'); end makes user-supplied YAML a lot less dangerous. I am going to poke this into a production application and see if anything breaks - it really really shouldn't

It breaks YAML deserialization in other places. You could enable and disable it on demand in the XML parser, but a more sensible solution is just to get YAML the hell out of the XML processor. Trying to make YAML safer is probably not the right approach.

It's meant to partially break YAML deserialization :) My apps do care about YAML, so I've an interest in cleaning this up. Is there some unintended consequence? You can still instantiate some Ruby classes (Regexp, Symbol etc.) in the YAML loader, or you can go through @@tagged_classes and pick out any other types you don't want.

But by taking out Object, YAML is only left with a whitelist of types that are safe, anything else will get turned into a YAML::DomainType.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#264

Post by mattbee »

charliesome wrote: post #230
mattbee wrote: post #204

Earlier quoted context omitted.

Hmm, looking around I am thinking that if you don't want random object instantiation, this monkey-patch: module YAML; @@tagged_classes.delete('tag:ruby.yaml.org,2002:object'); end makes user-supplied YAML a lot less dangerous. I am going to poke this into a production application and see if anything breaks - it really really shouldn't

This doesn't stop instantiating with !ruby/object

How doesn't it?

  irb(main):001:0> YAML::parse("!ruby/object:File 123").transform
  => #
  irb(main):002:0>  module YAML; @@tagged_classes.delete('tag:ruby.yaml.org,2002:object'); end
  => Object
  irb(main):003:0> YAML::parse("!ruby/object:File 123").transform
  => #
That's neutered what it'll do without causing the parser to blow up.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#265

Post by steveklabnik »

subsystem wrote: post #258
steveklabnik wrote: post #181

Earlier quoted context omitted.

I specifically asked them to elaborate on what could be made better.

There are specific things that could be said about the bug in question, like not being secure by default, but this doesn't fix the underlying problem. The development team should recognize that security is an important part of the project and act accordingly.

Seems reasonable. How did they not 'act accordingly' in this case? What should be different about the security process currently in place?

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#266

Post by steveklabnik »

lgleason wrote: post #248
steveklabnik wrote: post #242

Earlier quoted context omitted.

Aaron is the only person on core who is paid to work on Rails. (Among other things.) I am a committer, and part of my job is to work on open source. Ish. Other than that, it's everyone else's spare time, IIRC.

Wow, that actually gives me more confidence because we have people doing it because they feel passionate about it. Thanks for your hard work.... I've been busy with some other small OS projects (and stuff that pays the bills) but personally feel like I need to try to carve out some time this year to do something to contribute back to Rails.....

If I can help you help us somehow, please let me know.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#267

Post by steveklabnik »

nthj wrote: post #249
lambda wrote: post #236

Earlier quoted context omitted.

Less magic. The more magic, unexpected behavior you have when parsing untrusted input, the more likely you are to have security holes. Instead of building up some complex object based on untrusted input, the author of the application should specify the values and types expected, and the parser should parse those and nothing more. This would lead to much simpler code paths, as the user never has an object that has une…

This sounds a lot like strong_parameters, which (I believe) will be the default in Rails 4, and is only a gem install away from Rails 3.2 apps. https://github.com/rails/strong_parameters

It will be the default, yes.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#269

Post by jrabone »

mtrimpe wrote: post #86
jrabone wrote: post #44

Earlier quoted context omitted.

Who'd have thought it? Hugely dynamic language turns out to be difficult to audit or analyse for security issues. It was never about Java(C, C++) vs. Ruby despite what fanboys on either side made out. It was about conservative vs. devil-may-care. All that "convenience" and "it's so clean" came at the price of a whole load of code executed behind the scenes. You didn't write it, and the Gods of TDD preached that you d…

Wait, which one of us are you?

One of the troublemakers on a small island :-)

You've escaped anyway.

Re: Multiple vulnerabilities in parameter parsing in Action Pack

#270

Post by regularfry »

tptacek wrote: post #17
jerf wrote: post #15

Earlier quoted context omitted.

I don't speak Ruby. Can you or someone else be more precise about where that introduces the vulnerability? (Surely it isn't that YAML::load(content) can run arbitrary shell code?)

Calling YAML::load on attacker-controlled content in a Ruby app of any complexity is very bad news. As Ben and 'judofyr said: this is remote code execution.

Sorry, would you mind clarifying? Any Ruby app? So a Sinatra app which happened to YAML.load would also be at risk?
Post reply on HN