Software Integrity

 

Benefits of secure code review: Developer education

The value of code review, having been well-studied and documented, is generally accepted by most development managers, if not always by the developers themselves. While the primary goal of code review is to improve the quality of the code itself, a secondary goal is often to improve the knowledge and skills of the developers so they create better code in the future. If the developers can get past the ego blow that having their mistakes pointed out to them causes, their ego can then lead them to work toward beating the next review by using the lessons learned. The result is better code. The same can be true for secure code reviews if you account for the difference in methodology.

Lessons learned from code review

In Best Kept Secrets of Peer Code Review, Jason Cohen tells the story of reviewing a peer’s Java code and running across code like this: if (“integrate”.equals(str)) {…}

He could not understand why his peer checked for equal strings by using the static string rather than the more conventional str.equals(“integrate”). His peer explained that if he did str.equals(“integrate”) and str was null, an exception would occur; but, that “integrate”.equals(str) would always work, no matter whether str has a value or is null. In that brief interaction, he learned to remove an entire class of bugs from his own code and in his words: “I had fun doing it!” (1)

The ability to learn new ways of writing code to avoid errors in a standard code review like this is fairly easy. The developers interact with one another and ask questions. The developer who wrote the code explains what they did and why. The reviewers ask questions and offer suggested fixes or alternatives. Learning is inherent in the process and works both ways. The developers naturally teach each other during the review itself which results in better code and better developers.

Lessons learned from secure code review

Secure code reviews tend to go a bit differently. Rather than a group of developers interacting while looking at a limited amount of code, secure code review is often done by a lone reviewer or review team with security expertise, looking at a much larger set of code. Whether it is a manual review, a scan with a static analysis tool or a mixture of both, they usually happen away from the development team. This robs the secure code review process of the natural interaction involved with a regular code review and the lessons learned that go along with it.

Instead, the opportunity for developer education comes as the secure code review findings are explained. The reviewer shows the development team the findings and the education comes with a discussion of the issue and its common causes and an examination of examples within the code. Developers learn to avoid writing bad code as remediation is discussed.

This, however, lacks the power and immediacy of the interaction of a normal code review. Rather than active engagement in the review process, the developers may passively sit there waiting for the meeting to be over. An effective way around this is for the reviewer to display problem areas in code and guide the developers through the location and remediation of the security bugs at hand. This “spot the bug” session fosters interaction and actively engages the developers.

Once, while piloting secure code review with a development group with multiple applications, I presented the findings from the pilot application to the team responsible for it and to developers of the other applications. The “spot the bug” session had them all engaged and by time the other applications were reviewed, they had already begun to address the problems themselves.

Another effective way to engage the developers is through a demonstration of what happens when an attacker exploits their problems. Most people enjoy hacking demonstrations, and developers are no exception. The enjoyment even seems to overcome the ego hit that having your mistakes pointed out to you can cause. Developers can be so immersed in the demonstration that even as they groan “I wrote that code,” they forget to feel sorry for themselves. The demonstrations do not have to be elaborate, they just have to relate to the code under review.

Another educational benefit of the hacking demonstration it is that it exposes the developers to the attacker’s mentality. Developers tend to think about how to make things work, while attackers think about how to make things break. The mindset is different. Showing how an attacker subverts the developers’ assumptions is eye opening. With a web application I once reviewed, the developers did a very good job validating and properly handing everything they assumed was under an attacker’s control. The hacking demonstration showed them what their assumptions missed. The next review of the code came out much better than the first one had.

Secure code review – double the benefit

Secure code review can benefit you not just by making the code immediately under review more secure, but it can teach developers to write more secure code. This takes more effort than the natural learning which takes place in a more traditional review, but that effort can have the same effect. Once a developer learns a better way, their ego tends to drive them to write better code so to beat the reviewer next time. That drive to beat the review will improve the security of all of your code, not just the code under review.

Synopsys offers 3 market-leading static analysis solutions.

1 Jason Cohen, Best Kept Secrets of Peer Code Review (Smart Bear Inc, 2006), 91