81–90 of 240 posts
We've patched and fixed this on GitHub.
For reference, it's easy to determine if a Rails application is vulnerable to mass assignment attacks by attempting to set an attribute that is unlikely to exist (e.g., user[asdfg]). In the majority of cases a 500 error will be returned if the application is vulnerable.
We've patched and fixed this on GitHub.
Do you plan on notifying customers whose repos he accessed? This looks like more than just a minor breach if he was able to give himself admin rights and had read/write access to any repo.
Funnily, the first Diaspora release had the same issue and the devs were ridiculed and called noobs by a big part of the HN community and security "experts" wrote big posts about it. The different reaction here is interesting to say the least.
def destroy
@album = Album.find_by_id params[:id]
@album.destroy
[...]
end
user.albums.find_by_id params[:id]
We've patched and fixed this on GitHub.
Earlier quoted context omitted.
He's being an asshole. On the other hand, if the defaults in Rails lead even strong Rails developers into making mistakes, perhaps it is time for Rails to develop an "opinion" about this. I hope the asshole messenger doesn't obscure the importance of the message because people often push back harder against a message when it is delivered in an obnoxious manner by an obnoxious person.
I agree the message is the most important part of this despite the immature way he exposed this GitHub security issue. Rails can certainly adopt an 'opinion' regarding this issue, but if I think if we were to take a look around at heavy web frameworks today, we would see a very similar approach of "let the developer decide" when dealing with Model security and serialization of fields. These framework devs have no ide…
What everyone else is doing isn't a great justification--if decisions in Rails were based on what everyone else was doing, it would have been written in Java. In terms of Rails opinions, sensible defaults would be one that would suggest this should be rethought. In terms of rewrite-work, the Rails team didn't shy away from that with Rails 3, but I think the cross site scripting protection was worth the work. And even if the default is changed, nothing stops it from being a single line of code to turn it off.
Earlier quoted context omitted.
It's a pretty insane leap of logic to maliciously attack the Github website to prove your point regarding a framework hosted on that website. Politely contacting people about the vulnerability, rather than pulling that crap in an extremely public venue seems like the more mature, and less incredibly illegal way to go about things.
The "attack" clearly wasn't malicious, though probably immature. A malicious attacker would have been doing things like gaining access to users' private repos and stealing the code, or trying to sneak in harmful commits to a repo under a false name. This was at most a prank or a demonstration. So why this instead of responsible disclosure? The problem is that it was not really a GitHub issue, it's Rails having a gros…
* it provides a how to for other individuals to repeat the attack, in a public forum.
* it was made against an innocent third-party.
* I doubt steps were taken to contact the third-party.
* it was made on a Sunday morning. making it difficult to scramble and get a fix out the door.
"Clearly the Rails core team were not willing to consider any kind of changes to improve the situation"
The ticket was opened three days ago. Are you advocating that if an issue isn't resolved in an open-source project, in under a week, the individual raising the issue should be able to publicly attack anyone using the project?
Also, I'll put this gem of a quote out there:
"not only github is vulnerable this way - I found a lots of rails apps that are waiting for my hack! Yeah, it is only start" (mwahahahaha).
Earlier quoted context omitted.
Hmm? The vulnerability is in github's code (not open source) and not in rails.
The vulnerability is that Rails is insecure by default. That used to be the case for a lot of things, then finally people noticed how the real world works, and started fixing them. Apparently the Rails developers have actively resisted the lesson everyone else already learned.
There's very popular idiom in Rails development of updating model data from a form POST/PUT in one line of code in a controller:
my_object.update_attributes(params[:object])
Because many users follow this approach, it made this hack widely exploitable. You can either assign parameters piecemeal in the controller or explicitly set the attr_accessible attributes in the model. There's nothing inherent in Rails that caused this vulnerability, rather it was programming practices by developers.
Here's my proposal for improving the situation: https://gist.github.com/1974187 Merb's approach was to have mass assignment protection in the controller, and I personally think it's self-evident that it belongs there. Moving it into the controller will also make it easier to solve the tension between reducing the friction of getting up and running quickly and having good security defaults. In general, Rails' conventi…
Currently, I like to use a hack that automatically makes all models use attr_accessible with no allowed attributes. Until you override it with attr_accessible in your model, nothing is allowed, so you have to think hard about what should and shouldn't be accessible.
However, it causes real annoyance when not dealing with web input. Applying it to an existing project and fixing everything it breaks is super-annoying.
Earlier quoted context omitted.
> Responsible disclosure If you look at the bug report, the core Rails Dev Team basically said that they like the defaults the way that they are. They have/had no intention of changing the defaults, and are trying to push responsibility on to the developers using Rails to use sane config settings. Looks like the guy did report it and the response was: "Not our problem" / "Not an issue." He got frustrated and decided…
Rails bugtracker != notifying the people at github that they forgot to add a mass-assign protection.
Hence a plea to the framework to make this a default.
Here's my proposal for improving the situation: https://gist.github.com/1974187 Merb's approach was to have mass assignment protection in the controller, and I personally think it's self-evident that it belongs there. Moving it into the controller will also make it easier to solve the tension between reducing the friction of getting up and running quickly and having good security defaults. In general, Rails' conventi…