Is it really frowned upon to do: user.name = params[:user][‘name’] ?? Call me old fashioned, but this is called 'defensive coding' and should (in my opinion) be the norm when dealing with client-generated input. It might be more verbose and not 'The Rails Way', but update_attributes seems like too much magic for my paranoid taste.
How Homakov hacked GitHub & the line of code that could have prevented it
71–80 of 91 posts
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#72Is it really frowned upon to do: user.name = params[:user][‘name’] ?? Call me old fashioned, but this is called 'defensive coding' and should (in my opinion) be the norm when dealing with client-generated input. It might be more verbose and not 'The Rails Way', but update_attributes seems like too much magic for my paranoid taste.
It's not frowned upon, but it would require tighter coupling between the controller and the view...?
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#73Homakov kind of gave the impression he had discovered a previously unknown vulnerability in Rails. This is not the case. Rather, he discovered an instance in which one very prominent Rails app (Github) failed to implement a standard Rails security practice. For those not familiar with Rails, it boils down to this: You as the programmer need to use a security feature built into Rails called mass assignment security. I…
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#74Is it just me, or doesn't this sound very similar to SQL injection, only as applied to an ORM instead? That is, if my understanding is correct, they're taking user posted data and trivially turning it into a command to update data. This doesn't sound like a problem with Rails, in the same way that if I turn data I receive from the user straight into an SQL statement, the fact that people can abuse it isn't a problem…
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#75Is the solution given in the article any different than using this? config.active_record.whitelist_attributes = true Also, this isn't the first time someone's been bit by this: http://www.kalzumeus.com/2010/09/22/security-lessons-learned...
https://github.com/rails/rails/commit/06a3a8a458e70c1b6531ac...
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#76GitHub really needs to clearify if access to private repos was compromised, for how long and if such access would be traceable.
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#77Earlier quoted context omitted.
Github displays the username/avatar for a commit based on that commit's Author field, not the user that pushed the changes to the repository.
It's more that they display both. Unlike git core, github actually tracks "push" events to branches (git doesn't care, it only sees commits) as distinct from the commits they contain. So presumably it would have shown the rails developer pushing a change authored by Homakov.
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#78Homakov kind of gave the impression he had discovered a previously unknown vulnerability in Rails. This is not the case. Rather, he discovered an instance in which one very prominent Rails app (Github) failed to implement a standard Rails security practice. For those not familiar with Rails, it boils down to this: You as the programmer need to use a security feature built into Rails called mass assignment security. I…
I don't like the gist of your post. You are giving Rails way too much of a pass and assigning too much blame to GitHub. A framework should not be "insecure by design". Period. GitHub engineers are likely about as good as they get and still missed it. That's less an indictment of GitHub engineering and more so poor decision-making on the Rails side.
The point of my post was not to give Rails a pass, and I apologize for misleading if it came off that way. Rather, I was trying to clarify the situation for those who are less familiar with Rails. The discussions surrounding this issue (including Homakov's own words) seem to erroneously suggest that Homakov discovered a previously unknown vulnerability in Rails. I was merely clarifying that he instead found a vulnerability in a specific Rails app.
Now, it's a matter of opinion as to whether the Rails default should be called a "vulnerability." I say yes, but reasonable people can disagree. What's clear, though, is that no previously unknown vulnerabilities in the framework have been revealed.
Re: How Homakov hacked GitHub & the line of code that could have prevented it
#79Re: How Homakov hacked GitHub & the line of code that could have prevented it
#80Earlier quoted context omitted.
While I mostly agree that the Rails default should probably be changed, I think in fairness it's also worth pointing out that the Rails team does give fair warning about this issue. Specifically, anyone who reads the official Rails Security Guide back-to-back will know about this: http://guides.rubyonrails.org/security.html One might counter that nobody reads these guides back-to-back. I must admit I haven't read eve…
I'm not a big fan of the "this is documented" defense. If Microsoft left a network accessible default passworded Admin account in Windows Server but documented it and told people to change it, would that be okay simply because it was documented? Documentation is no panacea for bad defaults. Part of the OP's point (and he's absolutely right) is that this incident proves beyond a shadow of a doubt that just warning abo…
Of course, everyone should have really had a firewall anyway, so this was obviously cool right? After all, it's up to the user to secure their machine.
(Disclosure: That was sarcasm.)