I think it’s like .find_by_sql and .where and everywhere else Rails provides a way to write SQL fragments. You can compose strings and open yourself to SQL Injection, or you can have ActiveRecord insert sanitized parameters for you.
There’s no “vulnerability by default” in ActiveRecord’s handling of SQL snippets. There’s a right way and a wrong way. Likewise with mass assignment. There’s a right way to use it and a wrong way to use it. FWIW, I don’t use mass assignment, ever, because the “right” way offends my sensibilities by making me load up a model with authorization logic.
Rails is not vulnerable by default because Rails apps do not include mass assignment by default. It’s not like there’s a controller method called ‘update’ created if you don’t write one that uses mass assignment. You have to generate or write an update method, and if you do and you include a sql query, you have to get the sql fragments right. Likewise if you use mass assignment, you have to do attr_accessible right.
All that being said, I don’t like attr_accessible, I think it might have been brilliant at the time but a few years later I think we can revisit this problem with fresh eyes and a lot of experience as a community and do even better. Which by extension means that I don’t like mass assignment.
SO:
We probably agree that this feature should be taken out and shot, but are quibbling over which charge should be read off the indictment before giving the order to fire ;-)
> It’s not like there’s a controller method called ‘update’ created if you don’t write one that uses mass assignment. You have to generate or write an update method, and if you do and you include a sql query, you have to get the sql fragments right.
Yes, construction of raw SQL queries outside library code is a security hole. The same with writing browsers in C or C++. These bad design choices can be traced to countless security issues. The same for generating HTML using string manipulation. These are all stupid ideas, and until people realise that they're holes we will be stuck with endless security problems. And it's the coward's way out to hide behind the "sharper tools" nonsense. You don't hear chainsaw vendors wearing safety flaws as a badge of pride.
There’s no “vulnerability by default” in ActiveRecord’s handling of SQL snippets. There’s a right way and a wrong way. Likewise with mass assignment. There’s a right way to use it and a wrong way to use it. FWIW, I don’t use mass assignment, ever, because the “right” way offends my sensibilities by making me load up a model with authorization logic.
Rails is not vulnerable by default because Rails apps do not include mass assignment by default. It’s not like there’s a controller method called ‘update’ created if you don’t write one that uses mass assignment. You have to generate or write an update method, and if you do and you include a sql query, you have to get the sql fragments right. Likewise if you use mass assignment, you have to do attr_accessible right.
All that being said, I don’t like attr_accessible, I think it might have been brilliant at the time but a few years later I think we can revisit this problem with fresh eyes and a lot of experience as a community and do even better. Which by extension means that I don’t like mass assignment.
SO:
We probably agree that this feature should be taken out and shot, but are quibbling over which charge should be read off the indictment before giving the order to fire ;-)