4 ms·

>You are going to have problems with this whenever you are composing SQL statement with any type of user-provided data as part of the raw SQL string passed to t

>You are going to have problems with this whenever you are composing SQL statement with any type of user-provided data as part of the raw SQL string passed to the server. This is still, mathematically-speaking, a bug. The function is supposed to find a post by ID. If its implementation causes side effects or returns unexpected results for a certain subset of possible input data, then it doesn't conform to spec. This becomes a question of trust. Do you trust ActiveRecord/ORM of choice to be bug-free, or do you treat it as untrusted code and basically have to worry about the implementation of data persistence in your non-DB code even though that's what ORMs are supposed to abstract away? Why is that shit running around in your codebase anyway and not part of the ORM?

numbsafari 14y ago
I didn't say it wasn't a bug. It's a bug that indicates that ActiveRecord was written/designed in such a way that it trusts user-provided data to be executed. And, yes, I would fully expect to be able to trust my data-abstraction layer to be bug free. Since Rails seems to have this problem regularly, I can't trust it and therefore choose not to use it for those purposes. So, I think we agree here. --- Edit --- To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't (I don't know enough about the internals of rails to understand the patch). To my mind, the problem is that Rails should be treating any data passed to it as user-provided data, rather than trusting somebody who just took a 21-day "hacker college" class to do anything other than just pass along user-provided data. The framework should be implementing this kind of security in a consistent and reliable way, rather than trusting you. That's kinda what a framework is for.
- prodigal_erik 14y ago
  Not really a rubyist, but if you must do something as crazy as composing a query at runtime, isn't Object#tainted? the way to find out whether a string came from application code or from a user?
- rapind 14y ago
  While I sort of agree with your argument, I question whether or not it's our responsibility to protect the world from the 21-day "hacker college" crowd. I would welcome throwing an error instead however. There needs to be some sort of expertise cutoff and I think it's reasonable to expect in a web framework that it's user's are informed enough to avoid these sort of mistakes. Eventually the scissors become so safe that you can't cut anything with them.
  - numbsafari 14y ago
    I don't think we disagree. Developers should know about these issues and no framework will ever truly protect you from these kinds of mistakes. The question is, I think, is ActiveRecord an ORM that properly mitigates these issues. Anecdotally, it seems to have recurring problems with SQL injection.
- awj 14y ago
  > To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't They never tell you not to pass in user provided data. I have no idea where you got that conclusion, but you're obviously off and running with it. Quit spreading misinformation. The post includes a simple workaround for people who are not able to upgrade to a version that includes this security release. It's not a "don't do that" statement. > To my mind, the problem is that Rails should be treating any data passed to it as user-provided data It does. This is a bug. Please quit spreading misinformation.
  - numbsafari 14y ago
    This is the exact quote from the posting linked for this article: ---- Impacted code passes user provided data to a dynamic finder like this: Post.find_by_id(params[:id]) ---- It later tells you to apply the "to_s" function to the "user provided data" in order to avoid the problem. I'm unclear on how that is "misinformation".
    - awj 14y ago
      It tells you to do that in the "Workarounds" section when talking about how the vulnerability can be mitigated. At no point do they tell you not to pass user provided data to this method. The problem is an argument parsing bug that leads to user provided data being used as programmer provided data. Rails does not force SQL sanity off on the developer.
- SilasX 14y ago
  Whoa whoa whoa, let's be fair. As a graduate of one of these "hacker colleges" that used RoR [1], and who was a hobbyist programmer long before it, these magically generated function names (and plurals, etc) scared the hell out of me. In all my experience with programming, I had become accustomed to the mentality of "the compiler/interpreter expects identifiers to be exactly right; it doesn't figure out what you 'really' mean". So it was frustrating to see these auto-generated methods, as I couldn't see the rhyme or reason behind where these methods were coming from. Fortunately, I found work at a Django shop, where the framework is so much easier to follow and more explicit about how it does things. [1] devbootcamp.com, first cohort, Spring 2012, though it was actually more like 60 days with 40 days of instruction. I'm now employed as a developer and trusted with production code.

numbsafari 14y ago

I didn't say it wasn't a bug. It's a bug that indicates that ActiveRecord was written/designed in such a way that it trusts user-provided data to be executed. And, yes, I would fully expect to be able to trust my data-abstraction layer to be bug free. Since Rails seems to have this problem regularly, I can't trust it and therefore choose not to use it for those purposes. So, I think we agree here. --- Edit --- To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't (I don't know enough about the internals of rails to understand the patch). To my mind, the problem is that Rails should be treating any data passed to it as user-provided data, rather than trusting somebody who just took a 21-day "hacker college" class to do anything other than just pass along user-provided data. The framework should be implementing this kind of security in a consistent and reliable way, rather than trusting you. That's kinda what a framework is for.

prodigal_erik 14y ago
Not really a rubyist, but if you must do something as crazy as composing a query at runtime, isn't Object#tainted? the way to find out whether a string came from application code or from a user?
rapind 14y ago
While I sort of agree with your argument, I question whether or not it's our responsibility to protect the world from the 21-day "hacker college" crowd. I would welcome throwing an error instead however. There needs to be some sort of expertise cutoff and I think it's reasonable to expect in a web framework that it's user's are informed enough to avoid these sort of mistakes. Eventually the scissors become so safe that you can't cut anything with them.
- numbsafari 14y ago
  I don't think we disagree. Developers should know about these issues and no framework will ever truly protect you from these kinds of mistakes. The question is, I think, is ActiveRecord an ORM that properly mitigates these issues. Anecdotally, it seems to have recurring problems with SQL injection.
awj 14y ago
> To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't They never tell you not to pass in user provided data. I have no idea where you got that conclusion, but you're obviously off and running with it. Quit spreading misinformation. The post includes a simple workaround for people who are not able to upgrade to a version that includes this security release. It's not a "don't do that" statement. > To my mind, the problem is that Rails should be treating any data passed to it as user-provided data It does. This is a bug. Please quit spreading misinformation.
- numbsafari 14y ago
  This is the exact quote from the posting linked for this article: ---- Impacted code passes user provided data to a dynamic finder like this: Post.find_by_id(params[:id]) ---- It later tells you to apply the "to_s" function to the "user provided data" in order to avoid the problem. I'm unclear on how that is "misinformation".
  - awj 14y ago
    It tells you to do that in the "Workarounds" section when talking about how the vulnerability can be mitigated. At no point do they tell you not to pass user provided data to this method. The problem is an argument parsing bug that leads to user provided data being used as programmer provided data. Rails does not force SQL sanity off on the developer.
SilasX 14y ago
Whoa whoa whoa, let's be fair. As a graduate of one of these "hacker colleges" that used RoR [1], and who was a hobbyist programmer long before it, these magically generated function names (and plurals, etc) scared the hell out of me. In all my experience with programming, I had become accustomed to the mentality of "the compiler/interpreter expects identifiers to be exactly right; it doesn't figure out what you 'really' mean". So it was frustrating to see these auto-generated methods, as I couldn't see the rhyme or reason behind where these methods were coming from. Fortunately, I found work at a Django shop, where the framework is so much easier to follow and more explicit about how it does things. [1] devbootcamp.com, first cohort, Spring 2012, though it was actually more like 60 days with 40 days of instruction. I'm now employed as a developer and trusted with production code.