views:

211

answers:

3

I feel fortunate that I have been given an opportunity to improve code practices in my office by starting to implement some internal code reviews which could start out as some simple checklist. I need suggestions on tools and general tips.

I went to school for SoftE and understand the process of classic software development. I then worked at a big company making operating systems where we spared no cost reviewing code and looking for bugs. I was on a very good team and used checklists, diff tools, and participated in the mindset of scrutinizing code as a team.

Now I work in a nearly opposite office that makes web pages and no one reviews code, we don't even have CVS setup (though it was one blue moon ago before I got here). We recently got hacked and defaced on one of our minor servers due to some insecure code that allowed unrestricted file uploads. because of my experience I'm charged with coming up with the new security/quality review process which the boss likes because we can still bill for that time and he now sees that we need to do it. So I don't want to lose this opportunity because as much as I like the lack of structure here, we really could benefit from some.

My main question is what kind of tool would be good for putting up an internal review checklist for websites? We could build our own but reusing a tool is obviously better. Someone suggested Twiki; I haven't looked at it much. My main problem is Web Sites are not OSes, or even applications. They could have problems in apache, the db, php, even the js can reveal security holes, and these things are all interconnected like a web. It's hard to break up and I don't see one monolithic security checklist cutting it. I have been breaking it up based on each piece that interacts with another and coming up with the short list for that path, such as when going from PHP into a DB, remember to validate and escape input. From PHP to Javascript, don't list sensitive details in hidden fields or functions on the client side. That kind of thing.

I'll have to ultimately build something in-house that integrates all the aspects of web development. For now I'm focusing on security reviews, in the form of code reviews. I have to break it up by conceptual task because everyone will use it and some of us program, some just barely do some php, others are sysadmins, and we each have different things that need to be checked, especially between sysadmins and php developers.

+2  A: 

The first step would be to use some sort of version control to facilitate code review and to have incremental backups of all the coding. I've used svn and git with positive results

L. Cosio
A: 

Review board works well for us.

It supports wide variety of repositories like CVS, SVN, Perforce, Git, Mercurial etc.

obecalp
A: 

This may be an obvious thing and you may have already done it, but what about implementing a set of coding standards? This way you can "outlaw" certain types of code that result in security holes. When the code is reviewed it would be reviewed against these standards as well as other rules you have put in place. If nothing else, it could help eliminate simple coding mistakes that a lot of software engineers make such as no default in a case statement or putting an "=" instead of a "==" in an if statement.

Mark
We've tried, we just don't do it. We have to take the time to enforce the standard. I'll whip people into the habit of meeting to read code.
tkotitan