search

kandanapp / kandan

Kandan is an Open Source Alternative to HipChat
GNU Affero General Public License v3.0
2.72k stars 407 forks source link

Complete security overhaul on the application due to possible attack vec... #347

Closed scouttyg closed 10 years ago

scouttyg commented 10 years ago

...tors

I took yesterday and decided to do a security audit on the Kandan app, to look for other vectors. Other users have been awesome enough to submit their security issues, but I wondered if there were more problems than just surface deep.

There were many very worrisome issues that came up that should most likely be addressed immediately if the app is to become one that major organizations and such use. I'll explain below.

Warning! While I have made sure all tests pass, have tried to test on my own (signing in with multiple users, updating accounts, etc), this update should be tested multiple times by people before deploying to production, even if it is pulled in. I expected some quirky behavior here until we get any edge cases fixed.

scouttyg commented 10 years ago

I'm going to leave this issue / pull request open for a few days, so that people can comment or see any problems with it, then pull it in, unless everyone thinks it looks ok.

fusion94 commented 10 years ago

So the other question is that once this PR is merged do we stop dev work on Rails 3.x and focus on getting a Rails 4.x version out that becomes MASTER.

scouttyg commented 10 years ago

I definitely feel like Rails 4.x should be the next major feature (ie, before even having roles for channels and such). "Assuming" nothing goes wrong, my guess is after this is pulled in it might take a few hours or so to upgrade at least partially to Rail 4(IE, without adding strong parameters in yet). I'm still a little nervous at the state of Rails 4 gems, and the dependancies we might need to fix (hubot?) but I'd be willing to give it a try.

I think we should wait a week or so once we pull this in, however, just so that we can fix any issues that arise from such a big feature addition like this. We don't want to migrate the app to Rails 4 and start to get users with errors, and then have to wonder if it was from upgrading or from the security fixes.

It would also give us time to get any other small tweaks we have pulled in as well -- I know there are still a few mixed http/s issues and a few features that don't quite work perfectly.

My thoughts are -- pull this PR in by Wednesday (April 30th 2014), start migrating to rails 4 starting May 7th, 2014. Use a common branch we can both work on (rails4 or rails-4, not sure which you prefer), and when we pull it into master, make sure to have a rails-3.2 branch available so that we can backport any security fixes and also give people a version to use if for some reason they can't use rails-4. We'll also need to update the documentation in case rails 4 causes some special cases.

Let me know what you think.

scouttyg commented 10 years ago

Alright it's Wednesday! I'm pulling this in today. We'll leave a week open in case I broke anything, pull in any remaining pull requests if there, and then start Rails 4 development.