5 ms·

Perhaps I am misunderstanding the situation. Am I correct that the so-called “vulnerability” is that if you do a mass assignment from an untrusted source—form p

Perhaps I am misunderstanding the situation. Am I correct that the so-called “vulnerability” is that if you do a mass assignment from an untrusted source—form parameters—to a model without whitelisting attributes via attr_accessible, an attacker can write values to arbitrary attributes? If so, this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerability in apps written using a certain style, or perhaps a vulnerability by default in apps written using the generators and scaffolding baked into rails. But for guys like me who never use mass assignment, our applications are not vulnerable by default, correct? And likewise, for people who RTFM before using mass assignment, their applications are not vulnerable, correct? If I understand this, it is very different from something like the routes vulnerability a while back, where EVERY Rails app was vulnerable whether you liked it or not. Saying Rails has a security vulnerability here is like saying that ActiveRecord has a SQL Injection vulnerability because by default, find_by_sql allows you to compose queries out of strings, and if you don’t choose to use the correct form, you will be attacked by little bobby tables. (EDIT) To be clear, I like what the article says, I’m just not sure that the phrase “Rails has a security vulnerability” applies in this case, or that it is technically true that Rails is insecure by default.

jtimberman 15y ago
My point is that Rails does not take the high road of providing better, sane, secure defaults for people to build their applications. While the proposed solutions in the issues I linked to were denied, it doesn't mean there shouldn't be some solution added to Rails. This is definitely a case of convenience favored over security.
- raganwald 15y ago
  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 ;-)
  - deleted 15y ago
    [deleted]
  - benatkin 15y ago
    > 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. Or install a plugin and inherit one: https://github.com/josevalim/inherited_resources/blob/master/lib/inherited_resources/base_helpers.rb#L69 https://github.com/josevalim/inherited_resources/blob/master...
  - javascriptlol 15y ago
    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.
skeletonjelly 15y ago
Sure. But I feel like it's the job of a framework to hide away complexities to help the developer write code faster. I don't use Ruby/Rails but from my experience with ASP.NET MVC it seems that these helper functions exist for a reason. So why provide a tool that's inherently insecure? Caveat emptor?
- heymishy 15y ago
  Sure abstraction is good and speeds up workflow, but in this case, it was rolled into main without a lot of noise and essentially opened up developers to the situation without giving fair warning that this type of thing COULD happen. I'm not saying the blame lies solely with the Rails team - but the issue here is more with full disclosure and due diligence in communicating to the community.
- thenduks 15y ago
  On one hand you have `update_attributes` - IMO you should just never use this (seems raganwald agrees), but hey, it exists and it can be useful for simple stuff. So on the other hand is `attr_accessible` - You need to specify what is allowed to be mass-assigned if you want to use mass-assignment. This stuff is covered in the basic Rails Guides (http://guides.rubyonrails.org/security.html#mass-assignment http://guides.rubyonrails.org/security.html#mass-assignment). Saying that `update_attributes` is a security vulnerability doesn't really make sense... it's like if I said cars can accelerate to very unsafe speeds by default and this is a huge safely risk, ignoring the fact that cars also come with brakes :)
  - ryanbrunner 15y ago
    Pretty much every tutorial I can find on using controllers in Rails uses mass assignment, and the majority of them (including the beginners guide on guides.rubyonrails.org) don't mention attr_accessible or the security risk in allowing unrestricted mass assignment. We're telling people to RTFM, but the FM doesn't say a thing about how this is dangerous. If it was considered standard to avoid use of your brakes at all costs, and accelerate at maximum speed at all times, and this was reflected in the owner's manual - sure, I would think cars were unsafe.
timr 15y ago
"this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerability in apps written using a certain style, or perhaps a vulnerability by default in apps written using the generators and scaffolding baked into rails." Well, gee...so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Maybe it isn't a "vulnerability", per se, but it sure seems like a poor design choice. A "hardened" framework is going to make parameter whitelisting the default behavior, even if it's a little less convenient. If the github developers (who are presumably experienced) can make this mistake, then anyone can make this mistake.
- raganwald 15y ago
  so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM. If you want a framework that produces secure applications for inexperienced developers who do not read the fine manual, you are setting a laudable goal for Rails, but I hardly think that “Secure even for people who are 1. novices and 2. compound their ignorance by refusing to RTFM” is synonymous with “security problem.” If you open your history book to when Rails was first becoming popular, there were many people grumbling that because it was built on top of a dynamic language that it was vulnerable to all sorts of bugs caused by novice developers getting their types wrong. The argument at the time was that Rails provided a certain type of freedom and power at the expense of certain safeguards. The same argument was played out when people started noticing that monkey-patching run amuck caused certain problems. I agree with you that this is a design choice. If you want to say that you disagree with the choice as it was originally made, I agree with that too. I won’t say that I would have made a different choice back when that feature was first baked in, I did not write the framework. But here we are today, and it seems like a very good time to make a different choice.
  - 15y ago
jacques_chester 15y ago
You confusing mechanism and policy. Sure, Rails includes a mechanism that closes this security hole. But it requires additional effort to apply a more secure policy. The security baseline is not elevated by the mechanism. Historical experience has shown that no matter how excellent the mechanism, the practical security baseline is determined by the default policy. Windows NT has had, since inception, a far more sophisticated security mechanism than the default unix model. But that doesn't matter in the real world, because to retain backwards compatibility with Windows 95 (and through to DOS), almost no security policy was shipped by default until Vista. Was Windows insecure? According to your criteria, no. According to the actual real-world consequences, yes. The bottom line is that policy matters and that in this case, the rails crew have chosen a demonstrably bad policy.
- raganwald 15y ago
  The way you describe it, it sounds like there is a hole in every app and developers must close the hole to be secure. My claim is that Rails provides a completely optional tool, mass assignment, that opens a hole, and another completely optional tool that closes the same hole. And they document this well. The one tool can be said to have a poor policy, I agree with that. But Rails does not have a hole in it. I did not have to rush to “close” holes in the projects I’m responsible for when I read these posts because I hadn’t used the tool in the first place. What Rails has is a poorly designed tool. But if you’re going to say that if a developer can misuse the tool then Rails has a security problem, then I’m going to say that the exact same thing is true of ActiveRecord.
  - jacques_chester 15y ago
    > My claim is that Rails provides a completely optional tool, mass assignment, that opens a hole, and another completely optional tool that closes the same hole. And they document this well. The policy of the optional tool is broken. That's a bad thing. The entire point of a security baseline is to provide an attractor towards which design and code approach without resistance. If you want insecure, fine, but you'll have to go out of your way to get it. Rails is not doing that. > But if you’re going to say that if a developer can misuse the tool then Rails has a security problem, then I’m going to say that the exact same thing is true of ActiveRecord. And I would agree. The allure of Rails is how easy it is to get something going. The problem with Rails is how easy it is to get any old thing going. See also: every major PHP application ever written.
cbs 15y ago
>And likewise, for people who RTFM before using mass assignment, their applications are not vulnerable, correct? Brilliant point. While we're all talking about this issue, lets all team up, all of us here on HN, right now, to email php-internals and ask them to bring back register_globals and turn it on by default. It was a mistake on their part to disable it. After all, who doesn't read and follow all the documentation?
iamgilesbowkett 15y ago
But for guys like me who never use mass assignment, our applications are not vulnerable by default, correct? And likewise, for people who RTFM before using mass assignment, their applications are not vulnerable, correct? If I understand this, it is very different from something like the routes vulnerability a while back, where EVERY Rails app was vulnerable whether you liked it or not. Saying Rails has a security vulnerability here is like saying that ActiveRecord has a SQL Injection vulnerability because by default, find_by_sql allows you to compose queries out of strings, and if you don’t choose to use the correct form, you will be attacked by little bobby tables. I believe this is absolutely correct.
Peaker 15y ago
Convenience over safety is one of the repeated, expensive, wrong choices so endemic in the software world. Convenience is truly overrated. Inconvenience is cheap, lack of safety (which translates to security issues and bugs) is far far more expensive.
tylermenezes 15y ago
> or that it is technically true that Rails is insecure by default PHP learned their lesson a while ago with register_globals: if you need to specifically disable people from attacking your site in many places, it's inevitable that you'll miss one or two.
jamesgeck0 15y ago
I'm a just a fool with a simple Rails app and copy of _The Design of Everyday Things_, but this seems like a usability issue. The default scaffolding doesn't contain an attr_accessible property, so it's not visible to the user that there's something missing. There's also no feedback from Rails that your attributes are insecure; only feedback if one tries to change an attribute explicitly made non-accessible. Figuring out which attributes must be private is a problem possibly beyond the scope of core Rails, though. Perhaps a command like `rake routes` for publicly accessible model attributes might be of benefit.
- ryanbrunner 15y ago
  On top of that, Rails sometimes seems like it goes out of it's way to make attr_accessible a pain to use. I know more than once on my team someone has spent more than a few minutes trying to figure out why a model wasn't updating before they remembered to add a property to attr_accessible. In one case, we had a (rarely-used) field uneditable for a few weeks due to it not being included on the list. I really liked wycats proposal of moving this sort of thing to the controller - it's a much more logical place to look, and I do think there's a thing as a too-skinny controller - mapping inputs to model attributes is a perfectly sane thing for the controller to do IMO.
aneth 15y ago
Totally agree. The problem is the idiom of using mass assignment, which IMO should almost never be used. This is not a bug or a security hole in Rails, but an issue with programmers not paying attention. If Rails had no attr_accessible feature, it would be standard practice to always filter attributes in the controller, and no one would call this a rails issue - they would put the blame where it belongs: Github. Instead, because attr_accessible exists, people are flaming that it should be enabled by default.
smackay 15y ago
This short excerpt from the Hitchhikers Guide to the Galaxy seems appropriate: “`...You hadn't exactly gone out of your way to call attention to them had you? I mean like actually telling anyone or anything.' `But the plans were on display...' `On display? I eventually had to go down to the cellar to find them.' `That's the display department.' `With a torch.' `Ah, well the lights had probably gone.' `So had the stairs.' `But look you found the notice didn't you?' `Yes,' said Arthur, `yes I did. It was on display in the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying ”Beware of The Leopard".'"
generalk 15y ago
> But for guys like me who never use mass assignment, our > applications are not vulnerable by default, correct? Yes, if you never ever call `update_attributes` you're not vulnerable. This is, however, the most common way to perform model updates. > And likewise, for people who RTFM before using mass > assignment, their applications are not vulnerable, > correct? No! Absolutely not! The Github exploit is a prime example of this. Solid developers, who have obviously ReadTFM, can easily fall victim to this because the default is to be insecure! The fact is that Rails ships code generators that give you insecure code and put the onus on you to do something about it. I'm not absolving any developer of their responsibility to not shoot themselves in the foot. Ultimately, if this Github exploit had been used to do serious damage, Github, not the Rails core team, would have been responsible. It is the developer's responsibility to ensure that their code is as free of vulnerabilities as they can make it.

jtimberman 15y ago

My point is that Rails does not take the high road of providing better, sane, secure defaults for people to build their applications. While the proposed solutions in the issues I linked to were denied, it doesn't mean there shouldn't be some solution added to Rails. This is definitely a case of convenience favored over security.

raganwald 15y ago
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 ;-)
- deleted 15y ago
  [deleted]
- benatkin 15y ago
  > 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. Or install a plugin and inherit one: https://github.com/josevalim/inherited_resources/blob/master/lib/inherited_resources/base_helpers.rb#L69 https://github.com/josevalim/inherited_resources/blob/master...
- javascriptlol 15y ago
  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.

skeletonjelly 15y ago

Sure. But I feel like it's the job of a framework to hide away complexities to help the developer write code faster. I don't use Ruby/Rails but from my experience with ASP.NET MVC it seems that these helper functions exist for a reason. So why provide a tool that's inherently insecure? Caveat emptor?

heymishy 15y ago
Sure abstraction is good and speeds up workflow, but in this case, it was rolled into main without a lot of noise and essentially opened up developers to the situation without giving fair warning that this type of thing COULD happen. I'm not saying the blame lies solely with the Rails team - but the issue here is more with full disclosure and due diligence in communicating to the community.
thenduks 15y ago
On one hand you have `update_attributes` - IMO you should just never use this (seems raganwald agrees), but hey, it exists and it can be useful for simple stuff. So on the other hand is `attr_accessible` - You need to specify what is allowed to be mass-assigned if you want to use mass-assignment. This stuff is covered in the basic Rails Guides (http://guides.rubyonrails.org/security.html#mass-assignment http://guides.rubyonrails.org/security.html#mass-assignment). Saying that `update_attributes` is a security vulnerability doesn't really make sense... it's like if I said cars can accelerate to very unsafe speeds by default and this is a huge safely risk, ignoring the fact that cars also come with brakes :)
- ryanbrunner 15y ago
  Pretty much every tutorial I can find on using controllers in Rails uses mass assignment, and the majority of them (including the beginners guide on guides.rubyonrails.org) don't mention attr_accessible or the security risk in allowing unrestricted mass assignment. We're telling people to RTFM, but the FM doesn't say a thing about how this is dangerous. If it was considered standard to avoid use of your brakes at all costs, and accelerate at maximum speed at all times, and this was reflected in the owner's manual - sure, I would think cars were unsafe.

timr 15y ago

"this isn’t a security vulnerability in Rails. A new Rails application isn’t vulnerable by default. It’s a security vulnerability in apps written using a certain style, or perhaps a vulnerability by default in apps written using the generators and scaffolding baked into rails." Well, gee...so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Maybe it isn't a "vulnerability", per se, but it sure seems like a poor design choice. A "hardened" framework is going to make parameter whitelisting the default behavior, even if it's a little less convenient. If the github developers (who are presumably experienced) can make this mistake, then anyone can make this mistake.

raganwald 15y ago
so maybe some novice developers write an app using the generators and scaffolding baked into Rails, and as a result they get a security vulnerability that a more experienced developer might have avoided by doing extra work, and you say that's not a security problem in Rails? Young man, do not take that flippant tone with me (raps ruler on desk). Those novice developers failed to RTFM. If you want a framework that produces secure applications for inexperienced developers who do not read the fine manual, you are setting a laudable goal for Rails, but I hardly think that “Secure even for people who are 1. novices and 2. compound their ignorance by refusing to RTFM” is synonymous with “security problem.” If you open your history book to when Rails was first becoming popular, there were many people grumbling that because it was built on top of a dynamic language that it was vulnerable to all sorts of bugs caused by novice developers getting their types wrong. The argument at the time was that Rails provided a certain type of freedom and power at the expense of certain safeguards. The same argument was played out when people started noticing that monkey-patching run amuck caused certain problems. I agree with you that this is a design choice. If you want to say that you disagree with the choice as it was originally made, I agree with that too. I won’t say that I would have made a different choice back when that feature was first baked in, I did not write the framework. But here we are today, and it seems like a very good time to make a different choice.
- 15y ago

jacques_chester 15y ago

You confusing mechanism and policy. Sure, Rails includes a mechanism that closes this security hole. But it requires additional effort to apply a more secure policy. The security baseline is not elevated by the mechanism. Historical experience has shown that no matter how excellent the mechanism, the practical security baseline is determined by the default policy. Windows NT has had, since inception, a far more sophisticated security mechanism than the default unix model. But that doesn't matter in the real world, because to retain backwards compatibility with Windows 95 (and through to DOS), almost no security policy was shipped by default until Vista. Was Windows insecure? According to your criteria, no. According to the actual real-world consequences, yes. The bottom line is that policy matters and that in this case, the rails crew have chosen a demonstrably bad policy.

raganwald 15y ago
The way you describe it, it sounds like there is a hole in every app and developers must close the hole to be secure. My claim is that Rails provides a completely optional tool, mass assignment, that opens a hole, and another completely optional tool that closes the same hole. And they document this well. The one tool can be said to have a poor policy, I agree with that. But Rails does not have a hole in it. I did not have to rush to “close” holes in the projects I’m responsible for when I read these posts because I hadn’t used the tool in the first place. What Rails has is a poorly designed tool. But if you’re going to say that if a developer can misuse the tool then Rails has a security problem, then I’m going to say that the exact same thing is true of ActiveRecord.
- jacques_chester 15y ago
  > My claim is that Rails provides a completely optional tool, mass assignment, that opens a hole, and another completely optional tool that closes the same hole. And they document this well. The policy of the optional tool is broken. That's a bad thing. The entire point of a security baseline is to provide an attractor towards which design and code approach without resistance. If you want insecure, fine, but you'll have to go out of your way to get it. Rails is not doing that. > But if you’re going to say that if a developer can misuse the tool then Rails has a security problem, then I’m going to say that the exact same thing is true of ActiveRecord. And I would agree. The allure of Rails is how easy it is to get something going. The problem with Rails is how easy it is to get any old thing going. See also: every major PHP application ever written.

cbs 15y ago

>And likewise, for people who RTFM before using mass assignment, their applications are not vulnerable, correct? Brilliant point. While we're all talking about this issue, lets all team up, all of us here on HN, right now, to email php-internals and ask them to bring back register_globals and turn it on by default. It was a mistake on their part to disable it. After all, who doesn't read and follow all the documentation?

iamgilesbowkett 15y ago

But for guys like me who never use mass assignment, our applications are not vulnerable by default, correct? And likewise, for people who RTFM before using mass assignment, their applications are not vulnerable, correct? If I understand this, it is very different from something like the routes vulnerability a while back, where EVERY Rails app was vulnerable whether you liked it or not. Saying Rails has a security vulnerability here is like saying that ActiveRecord has a SQL Injection vulnerability because by default, find_by_sql allows you to compose queries out of strings, and if you don’t choose to use the correct form, you will be attacked by little bobby tables. I believe this is absolutely correct.

Peaker 15y ago

Convenience over safety is one of the repeated, expensive, wrong choices so endemic in the software world. Convenience is truly overrated. Inconvenience is cheap, lack of safety (which translates to security issues and bugs) is far far more expensive.

tylermenezes 15y ago

> or that it is technically true that Rails is insecure by default PHP learned their lesson a while ago with register_globals: if you need to specifically disable people from attacking your site in many places, it's inevitable that you'll miss one or two.

jamesgeck0 15y ago

I'm a just a fool with a simple Rails app and copy of _The Design of Everyday Things_, but this seems like a usability issue. The default scaffolding doesn't contain an attr_accessible property, so it's not visible to the user that there's something missing. There's also no feedback from Rails that your attributes are insecure; only feedback if one tries to change an attribute explicitly made non-accessible. Figuring out which attributes must be private is a problem possibly beyond the scope of core Rails, though. Perhaps a command like `rake routes` for publicly accessible model attributes might be of benefit.

ryanbrunner 15y ago
On top of that, Rails sometimes seems like it goes out of it's way to make attr_accessible a pain to use. I know more than once on my team someone has spent more than a few minutes trying to figure out why a model wasn't updating before they remembered to add a property to attr_accessible. In one case, we had a (rarely-used) field uneditable for a few weeks due to it not being included on the list. I really liked wycats proposal of moving this sort of thing to the controller - it's a much more logical place to look, and I do think there's a thing as a too-skinny controller - mapping inputs to model attributes is a perfectly sane thing for the controller to do IMO.

aneth 15y ago

Totally agree. The problem is the idiom of using mass assignment, which IMO should almost never be used. This is not a bug or a security hole in Rails, but an issue with programmers not paying attention. If Rails had no attr_accessible feature, it would be standard practice to always filter attributes in the controller, and no one would call this a rails issue - they would put the blame where it belongs: Github. Instead, because attr_accessible exists, people are flaming that it should be enabled by default.

smackay 15y ago

This short excerpt from the Hitchhikers Guide to the Galaxy seems appropriate: “`...You hadn't exactly gone out of your way to call attention to them had you? I mean like actually telling anyone or anything.' `But the plans were on display...' `On display? I eventually had to go down to the cellar to find them.' `That's the display department.' `With a torch.' `Ah, well the lights had probably gone.' `So had the stairs.' `But look you found the notice didn't you?' `Yes,' said Arthur, `yes I did. It was on display in the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying ”Beware of The Leopard".'"

generalk 15y ago

> But for guys like me who never use mass assignment, our > applications are not vulnerable by default, correct? Yes, if you never ever call `update_attributes` you're not vulnerable. This is, however, the most common way to perform model updates. > And likewise, for people who RTFM before using mass > assignment, their applications are not vulnerable, > correct? No! Absolutely not! The Github exploit is a prime example of this. Solid developers, who have obviously ReadTFM, can easily fall victim to this because the default is to be insecure! The fact is that Rails ships code generators that give you insecure code and put the onus on you to do something about it. I'm not absolving any developer of their responsibility to not shoot themselves in the foot. Ultimately, if this Github exploit had been used to do serious damage, Github, not the Rails core team, would have been responsible. It is the developer's responsibility to ensure that their code is as free of vulnerabilities as they can make it.