undefined | Better HN

0 pointssteveklabnik15y ago0 comments

Since I've already brought this up on Reddit...

For example: http://github.com/diaspora/diaspora/blob/master/app/controll...

There's no check to see if this is your photo or not. And before you mention it, the before_filter only checks if they're logged in, not permissions.

There are many, many similar things to this. Check out lib/encryptor.rb and shudder. I'm no security expert, but...

0 comments

10 comments · 5 top-level

erikpukinskis15y ago· 2 in thread

How is that a fundamental security flaw if there's an easy one line fix with no likely side effects?

brettbender15y ago

If it's that easy, why not just implement it in the first place and give people less of a reason to lambaste your application that supposedly centers around privacy?

erikpukinskis15y ago

It's ALPHA software. They're a few months in. They're releasing early and often. I think it's great that they're releasing it now and that people have the opportunity to work on it with them, rather than waiting for them to get everything up to production (or even beta) quality.

Maro15y ago· 2 in thread

How is that a fundamental problem? It takes two lines of code to fix.

This is an alpha release. People shouldn't be using it, that's all. They should've put in an artificial limitation like max. 2 users with max. 2 pics each per server to avoid people using it.

The whole thing being in Rails is much more of a turn-off for me.

ianb15y ago

If I saw "# FIXME: check perms" in that code I'd have said, sure, it's alpha, no biggie. But without any acknowledgement of what should be a totally obvious security hole, it makes one question what other holes there might be, and maybe they can all be reviewed away... but it can also lead to a lot of pain for a long time. But maybe the visibility of the project will help get that review done.

Realistically, though, so long as they don't get clever it might be fine. If they do clever things then review becomes very hard.

steveklabnikOP15y ago

It's not so much that it's hard to fix, it just demonstrates that they're either incompetent or not taking things seriously. It doesn't exactly inspire confidence for the future of the project.

aaronbrethorst15y ago· 1 in thread

Same thing is true with UsersController#update (http://github.com/diaspora/diaspora/blob/master/app/controll...). At least they had the good sense not to implement #destroy.

patio1115y ago

And if you want to wait a week or two, I will explain why that one function lets you comprehensively compromise any Diaspora user in any way you want. The team thinks it only changes their first name, last name, and profile (not login) email.

code_duck15y ago

That is pretty atrocious - I was very meticulous about issues like that (verifying ownership of resources before modifying them!) when I writing a what I knew to be an amateurish ecommerce system, with 9 months of PHP experience. No CS degree, nobody to help me. If they're doing stuff like this, the security situation is even worse than people are saying. If you can't get the huge grain issues even remotely right, the fine grained stuff is sure to slide right past you.

bradleyland15y ago

It looks like they're using Devise for authentication, but there is no attention to security beyond that. Devise is just the beginning of a robust security model. I haven't followed their mailing list/twitter/whatever, so I don't know what's going on internally, but here's how I'd rationalize this. Honestly, I don't care one way or another, but I enjoy playing the devil's advocate from time to time.

Rails 3 has been in beta for the better part of the summer. This means that many plug-ins (Devise included) haven't been able to keep up 100% compatibility. It's entirely possible that they've only implemented Devise for login authentication, but plan on expanding with something like Warden or Clearance for model/controller level security as the plug-ins come up to speed.

Our group started on a Rails 3 app back in March, and we learned early on that we should stick to our core app development, avoiding plug-in implementations until things stabilize. Hell, look at Bundler. The jump from 0.9.x to 1.0 broke our app deployment methodology between the beta3 release and Rails 1.0. We never even got around to beta4. When you develop using a beta framework that evolves quickly and breaks compatibility, you have to be careful where you place your efforts.

I'm not ready to lay down on the tracks and defend them here, but I don't think it's an impossible to assume that they just haven't addressed security yet. Maybe they haven't arrived at a framework choice. The fact that there is literally no security would seem to suggest this is true more so than if they had sparse security.

One aspect remains true, however. Anyone who deploys this and puts anything of value in it is asking to get body slammed.

j / k navigate · click thread line to collapse