Code Review

A great way to spot security problems is to have another developer or security expert review the code. Do it if you can.

We’ve looked a bit at tools to improve software security: checking the status of components, and analysing the code for basic security issues. But what about using teamwork – the ‘second mind on the problem’? How might we do that?

The classic, and very effective, answer is: code reviews. Our experts distinguished three kinds of code review, for different purposes:

 

  • Expert code review: A security expert reviews important sections of code for vulnerabilities.
  • Commercial review: A professional security company reviews your code against their vulnerability list.
  • Peer code review: One or more developers review another developer’s code, either in a special session or simply using pair programming.

Expert Code Review

If you have access to experts on secure coding, then code reviews are an effective way to use and transfer their expertise. The expert involved need not be a programmer; their skill lies in understanding where issues in code may cause problems, and in questioning the developers about such issues.

 

[We have] ‘software pen testing’, where you have some Subject Matter Experts who take a piece of code and review that in detail, and work with the developers in tandem, looking at the code and saying “we think this is really high risk, we really want to look at this.”  And then somebody goes through that code with the mindset of “how do I exploit the code” (Security team lead, Operating system supplier)

 

Such reviews serve other purposes as well as improving the section of code reviewed: they are a good way of teaching secure coding to the developers; and they can act as a ‘nudge’ to remind programmers of the importance of security.

 

Getting time from external experts of this kind tends to be difficult, and can be expensive for your organisation so do triage code according to its security risk and use this kind of review only for the most important.   

 

If there is something in the application that is a particularly sensitive area … you probably want a security guy to come and sit for a few days and have a look at it,  someone who can work with the develop team but who hasn’t been part of that development team, in a way. So somebody who can ask the stupid question - I think that is really important, to bring in an external person. (Consultant trainer, Security specialist)

Commercial Code Review

The widely used equivalent to Penetration Testing for a mobile app is an external security code review. 

 

Many of the companies who perform Penetration Testing also do this kind of code review; they gather lists of known security issues found in apps, with mitigations for each, and then review the provided code to look for those security issues.

Peer Code Review

For most teams it’s much more practical to do internal code reviews – to have one or more other team members as the reviewers.  Some teams even have a formal review process, with separate review meetings delivering lists of defects for a developer to fix.

 

It is in the culture. We do reviews; we always have to do reviews. We set things up – and it is not regarded as a second class. (Security expert, Security and military supplier)

 

However, formal code review is regarded as relatively expensive, and others use more informal peer review, using pair programming. 

 

We do a lot of peer review. So we will do ‘buddy builds’ where people will work together for a week, and work on their own, and work together for a week. (Security team lead, Operating system supplier)

 

Research suggests that pair programming and code reviews have a productivity benefit rather than a cost: when evaluated over a long period, teams that do code reviews are more productive than those who do not. But programmers tend not to like code reviews; it is uncomfortable criticising other people’s code, and even more uncomfortable when it’s your own code being criticised. It is surprisingly difficult to set up Code Reviews and keep them going if you do not already have the kind of culture that supports reviewing. And if you either work alone, or effectively alone in that you have no colleagues working on the same project, peer Code Review may not be an option for you. 

Recommendation

Thus, our recommendation is to get security experts to code review key parts of the system if you can – that is, if they have the expertise and willingness to do it. If your budget allows it, consider even paying for a commercial review. 

 

Considering peer code reviews, though, our recommendation is not to set yourself up to fail – it’s demotivating for everyone – so unless the development team are really keen or your process already supports it, avoid trying to set up a system of code reviews. Do consider pair programming, though, especially for sensitive parts of the code.

Resources

A good resource on code reviews is the OWASP online handbook

 

Surprisingly few books tackle the topic of code reviews. The original was Software Inspections by Tom Gilb and Dorothy Graham. 

 

Pair programming was originally introduced on the C2 Wiki. Any books on Agile Development will have some discussion on the subject. There’s a friendly introduction on WikiHow .