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

SQL Injection Vulnerability in Ruby on Rails; affects all versions

groups.google.com

Post reply on HN
Print view View original

151–160 of 220 posts

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#151

Post by Groxx »

venus wrote: post #132

The overreaction on this page is ridiculous. Has anyone actually read the steps required to exploit this vulnerability? You do know that to be able to exploit it you have to know the application's secret key, so you can create your own malicious encrypted session cookie that includes hashes instead of strings for the auth token lookup? You do know that if someone has your app's secret key they can just write whatever…

Have you tried it? I can do this:

  params[:id] = {:select => "select * from users where admin = 1 limit 1; --"}
  User.find_by_id(params[:id]) # => finds the first admin
  # generated SQL:
  # SELECT * from users where admin = 1 limit 1; -- select * FROM `users` WHERE (`users`.`id` IS NULL) LIMIT 1
Seems like a bigger bug than you seem to think it is. Merely accepting JSON and not calling .to_i is enough for me to select any user.

-- edit: maybe nvm on that, it requires an actual symbol, not a string key with a hash with indifferent access, like you'd get from JSON. Unless someone knows a way to get a symbol into a JSON or form-encoded field? Though you're vulnerable to this specifically if you use '.symbolize_keys' anywhere before it's passed in.

-- edit2: and I see I'm late to the game anyway, thanks tenderlove :)

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#152

Post by eric_h »

eclipxe wrote: post #82
ultimoo wrote: post #73

Earlier quoted context omitted.

True, but :find_by_id is closer to :where in that it doesn't raise exceptions upon not finding a record. I've seen it being used in a few places, especially in front another scope, like `current_user.posts.find_by_id`.

If you have an invalid id coming in, wouldn't you generally want an Exception? What is the use case where the app would send an id that does not exist, but you would not want to fail?

While the example given here is :find_by_id, the reality of the isse is that it affects the dynamic finder methods. This means that it affects the :find_by_any_column_in_the_table methods. The only reasonable situation in which to throw an error in those methods is when you don't hit a column, not when you don't find a record, IMO.

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#153

Post by paulgb »

up_and_up wrote: post #143
chrismsnz wrote: post #138

Earlier quoted context omitted.

That's a bug/exploit in Authlogic, a third party library for rails, not rails. However, that bug/exploit is based on the rails vuln and was patched in authlogic exactly how the Rails report instructed people to work around it (casting the parameter to a string). Reading the actual report linked in the OP you'll see that generic and boilerplate code (e.g. the extremely common pattern: "Post.find_by_id(params[:id])") i…

"This leaves persist_by_session open to sql attacks (such as logging in as any user), if a malicious user can write their own rails session cookie ( if they have the rails secret_token )." The key is: "if they have the rails secret_token" The secret token is autogenerated when the application is initially bootstrapped. Here is more information about it from any config/initializers/secret_token.rb file: # Your secret…

354 matches.

https://github.com/search?utf8=%E2%9C%93&q=secret_token....

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#154

Post by eclipxe »

Groxx wrote: post #151
venus wrote: post #132

The overreaction on this page is ridiculous. Has anyone actually read the steps required to exploit this vulnerability? You do know that to be able to exploit it you have to know the application's secret key, so you can create your own malicious encrypted session cookie that includes hashes instead of strings for the auth token lookup? You do know that if someone has your app's secret key they can just write whatever…

Have you tried it? I can do this: params[:id] = {:select => "select * from users where admin = 1 limit 1; --"} User.find_by_id(params[:id]) # => finds the first admin # generated SQL: # SELECT * from users where admin = 1 limit 1; -- select * FROM `users` WHERE (`users`.`id` IS NULL) LIMIT 1 Seems like a bigger bug than you seem to think it is. Merely accepting JSON and not calling .to_i is enough for me to select an…

You're in the console. A param hash from a get or post won't have symbols for keys.

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#155

Post by eclipxe »

eric_h wrote: post #152
eclipxe wrote: post #82

Earlier quoted context omitted.

If you have an invalid id coming in, wouldn't you generally want an Exception? What is the use case where the app would send an id that does not exist, but you would not want to fail?

While the example given here is :find_by_id, the reality of the isse is that it affects the dynamic finder methods. This means that it affects the :find_by_any_column_in_the_table methods. The only reasonable situation in which to throw an error in those methods is when you don't hit a column, not when you don't find a record, IMO.

I agree on this point

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#156

Post by eclipxe »

RegEx wrote: post #89
eclipxe wrote: post #82

Earlier quoted context omitted.

If you have an invalid id coming in, wouldn't you generally want an Exception? What is the use case where the app would send an id that does not exist, but you would not want to fail?

nonexistent does not necessarily imply invalid. I don't use rails, so take my ignorance into account, but I'd hate to catch exceptions all the time instead of just checking for falsey values. Ex: "Enter your student ID" s = Student.find_by_id(params[:id]) if s # do stuff else # do other stuff end vs begin s = Student.find_by_id(params[:id]) # do stuff rescue # do other stuff end

This is a good example, however I would argue that it is bad design to have a record's ID used for user facing lookups. For other parameters, sure (find_by_name, etc) a false would be okay. Just a matter of taste really.

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#157

Post by SoftwareMaven »

paulgb wrote: post #153
up_and_up wrote: post #143

Earlier quoted context omitted.

"This leaves persist_by_session open to sql attacks (such as logging in as any user), if a malicious user can write their own rails session cookie ( if they have the rails secret_token )." The key is: "if they have the rails secret_token" The secret token is autogenerated when the application is initially bootstrapped. Here is more information about it from any config/initializers/secret_token.rb file: # Your secret…

354 matches. https://github.com/search?utf8=%E2%9C%93&q=secret_token....

That's an interesting issue (Django has the same issue with its SECRET_KEY). If you have an open-source project that utilizes these kinds of technologies, you need to keep your secret key secret.

As it says in the Django settings:

"Make this unique, and don't share it with anybody."

Your web application's security depends on it!

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#158

Post by aaronblohowiak »

charliesome wrote: post #114
aaronblohowiak wrote: post #113

Earlier quoted context omitted.

This is not a rubyism, it is a Railsism, they wrote a method called extract_options! and use it everywhere to get this kind of behavior. This is not how vanilla ruby works.

If it wasn't a rubyism, why is there syntax sugar for passing a hash as the last positional parameter?

the sugar exists for passing it, but not for receiving it, and to get around this, rails uses splat (varargs) and extract_options! to get the last positional parameter behavior for vararg methods.

vanilla ruby:

  def find_by_name(name, options = {})
  ...
  end
rails:

  def find_by_name(*args)
    opts = args.extract_options!
    name = args.shift
    ...
  end
(note, this is not how the dynamic finders actually work, i just wanted to illustrate the difference in the calling and definition.)

So, ruby doesn't include support for varargs with last positional parameter for options, rails builds that in. The fact that it is variable arity is very important -- in fact, that is the root of the present issue. The patches now check the number of arguments.

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#159

Post by [deleted] »

jtchang wrote: post #145
venus wrote: post #132

The overreaction on this page is ridiculous. Has anyone actually read the steps required to exploit this vulnerability? You do know that to be able to exploit it you have to know the application's secret key, so you can create your own malicious encrypted session cookie that includes hashes instead of strings for the auth token lookup? You do know that if someone has your app's secret key they can just write whatever…

It is not an overreaction. This is a much more subtle SQL injection. I believe the takeaway is that too much magic is a bad thing when it obscures the underlying behavior. Post.find_by_id( ) accepts an argument. Here are some normal assumptions: 1. It might only take a number 2. The method might coerce it to a string or integer for you 3. The method might not coerce it. 4. The method might throw an error if it isn't…

[deleted]

Re: SQL Injection Vulnerability in Ruby on Rails; affects all versions

#160

Post by mikeryan »

paulgb wrote: post #153
up_and_up wrote: post #143

Earlier quoted context omitted.

"This leaves persist_by_session open to sql attacks (such as logging in as any user), if a malicious user can write their own rails session cookie ( if they have the rails secret_token )." The key is: "if they have the rails secret_token" The secret token is autogenerated when the application is initially bootstrapped. Here is more information about it from any config/initializers/secret_token.rb file: # Your secret…

354 matches. https://github.com/search?utf8=%E2%9C%93&q=secret_token....

did you even look at what was matched?

Change the cookie secret token at config/initializers/secret_token.rb

Create a config/initializers/secret_token.rb file:

That will rename your app in the following files: ... config/initializers/secret_token.rb

Change your Application’s Secret Token ...

Change the secret token at /config/initializers/secret_token.rb

Those are the first six items in order and the trend continues at least through the first page of results.

Post reply on HN