3 ms·

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 d

by 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.
- caioariede 14y ago
  Can't see how it's different from: (in a manager) def find_by_foo(self, arg): return self.filter(foo=arg)[:1]
  - riffraff 14y ago
    please reread what I or others wrote, cause I don't understand why you can't see the difference. What you wrote is exactly what I wrote that a finder method _could be but it's not and that is the issue_

by elbear 14y ago

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.
caioariede 14y ago
Can't see how it's different from: (in a manager) def find_by_foo(self, arg): return self.filter(foo=arg)[:1]
- riffraff 14y ago
  please reread what I or others wrote, cause I don't understand why you can't see the difference. What you wrote is exactly what I wrote that a finder method _could be but it's not and that is the issue_