7 ms·

Btw, this applies for my (Python) Flask apps using MongoDB ORMs. They escape the inputs. What the hell is going on with ActiveRecord?

charliesome 14y ago
The main issue stems from Ruby using the last positional parameter to pass a hash representing keyword arguments. This means if there's only one parameter, and someone can sneak a Hash in there where you weren't expecting it (params parsing, request body parsing, etc) then they can end up passing dodgy 'keyword arguments' into your method call.
- elbear 14y ago
  In Python, the parameter escaping is done at the level of the database driver not the ORM. Isn't this the case with Ruby? Of course, you could use the driver incorrectly to risk SQL injection, but that is a very obvious mistake that no experienced developer would make.
  - riffraff 14y ago
    the ORM escapes the parameter but it lets you specify bits of SQL by hand (think "select foo, myfunc(bar) as BAZ, joineds.quux as quux"). In theory when you do that you have already given up on letting the framework handle it for you, and you must take care of not feeding raw user input as the select code, for example. The issue here is that the option to do this is exposed in a functionality where people do not expect it (dynamic finders) and thus people may be passing risky input there.
    - elbear 14y ago
      Well, the Django ORM also allows you to write SQL by hand and if you make a mistake you can fall pray to SQL injection, so I'm assuming that there's something different about this exploit. From what I understand the current issue appears because the person who implemented the faulty method uses SQL directly and doesn't pass the parameters separately. In Python, you would do something like this: execute('select name, age from employees where id=?', (params['id'],)) This passes the id as the second argument to the execute function. If you do this, on the other hand, you open yourself to SQL injection, because %s is replaced with params[id] and no escaping is done: execute('select name, age from employees where id=%s' % params['id'])
      - riffraff 14y ago
        the thing that is different is that the raw sql facilities are made explicitly available not through the common methods that expect them, but through something else. The orm supports building the sql piecemeal, e.g find(select: "name, foo(bar) as baz", conditions: 'x=y', limit: 3) this is a small step above a raw execute, and obviously ugly and low level. Also a somewhat obsolete practice, since for a few years you could write it as a composition of calls select("name, foo(bar) as baz"). where('x=y'). limit(3) Anyway the functionality is there to compose SQL via bits using an hash of parameters, moving on. Now remember that ruby <2.0 does not support keyword arguments, so the common practice is to use one normal argument with an hash value wich contains the keyword args. Rails has this (antipattern imo) of accepting arguments in a dozen way for some methods e.g. find(1) find(:first) find([1]) find(1,limit: 1) let us not argue whether this is good, it's there. And AR has dynamically generated finders (which Django does not have AFAIR). One would expect the dynamically generated finder to be doing def find_by_foo arg where(foo, arg).limit(1) end but in reality it does def find_by_foo *args many_options = args.extract_options! opts = combine_with_foo_handling(many_options) find(opts) end and here you get the problem that you may be unknowingly passing an hash object wich builds sql piecemeal. Notice that, as others already pointed out, usually as a user you shouldn't be able to create custom objects of the kind that exploits this issue (an hash with symbols as keys) unless your have other vulnerabilities already.
        jsmeaton 14y ago
        Just so you know, Django has a sort of dynamic finder implemented with kwargs in the lookup Post.objects.filter(some_field_name=some_value)
        riffraff 14y ago
        I know that, but the equivalent AR is `Post.where(somefield: somevalue)`, dynamic finders are another thing. With no obvious value over the former that I may think of anyway, I think they are mostly there for historical reasons.
- codewright 14y ago
  You can pass a hash (dictionary) to represent kwargs in Python. This seems like bad engineering fundamentals in the design of ActiveRecord for it to be perpetually subject to this sort of thing.
  - charliesome 14y ago
    > You can pass a hash (dictionary) to represent kwargs in Python. Eh, sure but you need to explicitly pass the dict as a kwarg with a double-asterisk, or else it's just a normal positional parameter. In Ruby prior to 2.0, there is no formal concept of kwargs, so there is no distinction between passing a Hash as the last positional parameter and passing kwargs. This is the root problem, and I look forward to it going away when everyone moves to 2.0.
- andrewcooke 14y ago
  i don't know ruby, so forgive the dumb question, but doesn't that mean that a single syntax has two different semantics? what if you want to pass a hash to a method as the last parameter? what decides whether it is treated as a hash or keywords?
  - charliesome 14y ago
    There are no 'keyword arguments', however Ruby does provide syntax sugar for options hashes. For example, this: my_method(1, 2, three: 3, four: 4) Is the same as this: my_method(1, 2, { three: 3, four: 4 }) Which can be picked up by the method like this: def my_method(one, two, opts) three = opts[:three] four = opts[:four] puts one, two, three, four end
- aaronblohowiak 14y ago
  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.
  - charliesome 14y ago
    If it wasn't a rubyism, why is there syntax sugar for passing a hash as the last positional parameter?
    - aaronblohowiak 14y ago
      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.

charliesome 14y ago

The main issue stems from Ruby using the last positional parameter to pass a hash representing keyword arguments. This means if there's only one parameter, and someone can sneak a Hash in there where you weren't expecting it (params parsing, request body parsing, etc) then they can end up passing dodgy 'keyword arguments' into your method call.

elbear 14y ago
In Python, the parameter escaping is done at the level of the database driver not the ORM. Isn't this the case with Ruby? Of course, you could use the driver incorrectly to risk SQL injection, but that is a very obvious mistake that no experienced developer would make.
- riffraff 14y ago
  the ORM escapes the parameter but it lets you specify bits of SQL by hand (think "select foo, myfunc(bar) as BAZ, joineds.quux as quux"). In theory when you do that you have already given up on letting the framework handle it for you, and you must take care of not feeding raw user input as the select code, for example. The issue here is that the option to do this is exposed in a functionality where people do not expect it (dynamic finders) and thus people may be passing risky input there.
  - elbear 14y ago
    Well, the Django ORM also allows you to write SQL by hand and if you make a mistake you can fall pray to SQL injection, so I'm assuming that there's something different about this exploit. From what I understand the current issue appears because the person who implemented the faulty method uses SQL directly and doesn't pass the parameters separately. In Python, you would do something like this: execute('select name, age from employees where id=?', (params['id'],)) This passes the id as the second argument to the execute function. If you do this, on the other hand, you open yourself to SQL injection, because %s is replaced with params[id] and no escaping is done: execute('select name, age from employees where id=%s' % params['id'])
    - riffraff 14y ago
      the thing that is different is that the raw sql facilities are made explicitly available not through the common methods that expect them, but through something else. The orm supports building the sql piecemeal, e.g find(select: "name, foo(bar) as baz", conditions: 'x=y', limit: 3) this is a small step above a raw execute, and obviously ugly and low level. Also a somewhat obsolete practice, since for a few years you could write it as a composition of calls select("name, foo(bar) as baz"). where('x=y'). limit(3) Anyway the functionality is there to compose SQL via bits using an hash of parameters, moving on. Now remember that ruby <2.0 does not support keyword arguments, so the common practice is to use one normal argument with an hash value wich contains the keyword args. Rails has this (antipattern imo) of accepting arguments in a dozen way for some methods e.g. find(1) find(:first) find([1]) find(1,limit: 1) let us not argue whether this is good, it's there. And AR has dynamically generated finders (which Django does not have AFAIR). One would expect the dynamically generated finder to be doing def find_by_foo arg where(foo, arg).limit(1) end but in reality it does def find_by_foo *args many_options = args.extract_options! opts = combine_with_foo_handling(many_options) find(opts) end and here you get the problem that you may be unknowingly passing an hash object wich builds sql piecemeal. Notice that, as others already pointed out, usually as a user you shouldn't be able to create custom objects of the kind that exploits this issue (an hash with symbols as keys) unless your have other vulnerabilities already.
      - jsmeaton 14y ago
        Just so you know, Django has a sort of dynamic finder implemented with kwargs in the lookup Post.objects.filter(some_field_name=some_value)
        riffraff 14y ago
        I know that, but the equivalent AR is `Post.where(somefield: somevalue)`, dynamic finders are another thing. With no obvious value over the former that I may think of anyway, I think they are mostly there for historical reasons.
codewright 14y ago
You can pass a hash (dictionary) to represent kwargs in Python. This seems like bad engineering fundamentals in the design of ActiveRecord for it to be perpetually subject to this sort of thing.
- charliesome 14y ago
  > You can pass a hash (dictionary) to represent kwargs in Python. Eh, sure but you need to explicitly pass the dict as a kwarg with a double-asterisk, or else it's just a normal positional parameter. In Ruby prior to 2.0, there is no formal concept of kwargs, so there is no distinction between passing a Hash as the last positional parameter and passing kwargs. This is the root problem, and I look forward to it going away when everyone moves to 2.0.
andrewcooke 14y ago
i don't know ruby, so forgive the dumb question, but doesn't that mean that a single syntax has two different semantics? what if you want to pass a hash to a method as the last parameter? what decides whether it is treated as a hash or keywords?
- charliesome 14y ago
  There are no 'keyword arguments', however Ruby does provide syntax sugar for options hashes. For example, this: my_method(1, 2, three: 3, four: 4) Is the same as this: my_method(1, 2, { three: 3, four: 4 }) Which can be picked up by the method like this: def my_method(one, two, opts) three = opts[:three] four = opts[:four] puts one, two, three, four end
aaronblohowiak 14y ago
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.
- charliesome 14y ago
  If it wasn't a rubyism, why is there syntax sugar for passing a hash as the last positional parameter?
  - aaronblohowiak 14y ago
    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.