Security research, news and guidance

A checklist approach to security code reviews

December 10, 2009  |  Written by Security Ninja  |   Application Security   |   16 Comments

Hi everyone,

A question I often get asked about manual security code reviews (manual source code review) is how can a manual review process be clear, consistent and repeatable? I’m going to share some of the things I do to try and make sure security code reviews aren’t seen as a black box process filled with security professionals practicing some kind of ninja skills on the source code.

To help me with security code reviews I have created a checklist that started off quite small and has grown to contain around 60 different items. I know some people don’t like the checklist approach but I think it is great for providing clarity and repeatability to the security code review process. What I find interesting and at the same time disappointing is that a quick Google for security code review checklist returns very few results that are useful. The only decent checklists for security code reviews thrown up by Google are those from Microsoft which are understandably written for the .NET world.

This reminded me of my reasons for creating the principles of secure development because again there is no one making this easy to understand. The OWASP Code Review Guide is a good example of a project which aims to help people but misses out the simple things like a list of checklist items which would really help people. The guide provides some very useful information on what to look for when you are carrying out security code reviews but its 200+ pages is a daunting prospect for someone new to the application security world. The guide is just another area of application security where the KISS (Keep It Short and Simple) approach should be followed rather than a constant desire to add more.

I have seen great traction and support for the principles approach to secure web application development that we have created so I have attempted to take the same approach to security code reviews. The checklist includes some questions that relate to security items I feel should be contained in application design documents as well as things to look for in the source code. These checklist items should ideally be reviewed when the application design document is produced rather than at the end of the SDLC process (this assumes that security is integrated throughout your SDLC).

I will post the security code review checklist items for one or two secure development principles each week for the next 4 or 5 weeks with an explanation of each checklist item so that you can use them in your own review process.

You will need to choose one of three different answers for each checklist item: YES, NO or N/A. They are worded in such a way that a NO answer is a security failure (this might be different in your own environment) and an N/A should be accompanied by a short description as to why you feel it is N/A.

This week’s blog post will cover the checklist items for Input and Output Validation. The checklist items based on the content of design documents might not be applicable for every security code review.

Input and Output Validation

We have 9 checklist items for Input and Output Validation which you can see below:

Check1

This first checklist question relies on the design document including security focused data flow diagrams such as those created for use in threat models. The entry points and trust boundaries identified on these diagrams can help you pinpoint areas of risk.

Check2

The diagrams mentioned in the previous checklist item can also help with the second item on our checklist. We are looking at the data flow diagrams and identifying points where data traverses trust boundaries. We should review these points in the source code to ensure that data validation has been implemented correctly.

Check3

This item requires the reviewer to identify how the application performs input validation. The input validation in the application should check the content as well as the minimum and maximum lengths for all data types received and processed.

Check4

An often overlooked step in application security is the canonicalization of data prior to performing input validation. This can allow encoded attack strings to bypass the input validation functions you have implemented.

Check5

Applying input validation on only the client or server side could allow vulnerabilities to occur in your application. The application should always enforce data validation on both the client and server side.

Check6

If the application receives XML requests it should validate the XML against an XML schema.

Check7

This checklist item is a reminder that any values that an external user could manipulate must be validated.

Check8

We should always be aware of where the output from our application is going to end up (i.e. in a URL) and apply the correct type of encoding. This checklist item will identify the points in the application where externally supplied input is used as output from our application and ensure that the correct encoding has been applied.

Check9

We should always be aware of where the output from our application is going to end up (i.e. in a URL) and apply the correct type of encoding. This checklist item will identify where any data is output from our application and ensure that the correct encoding has been applied.

These checklist items will allow you to carry out a security code review which checks that the input and output validation secure development principles have been implemented correctly.

I would love to hear any feedback you have on these checklist items as well as any additional items you think should be in the checklist.

SN

This entry was posted on December 10, 2009 at 12:21 pm and is filed under Application Security . You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

16 comments   >

  1. aex says:

    Excellent post, thanks!

  2. Pingback: Checklists for Application Security « Supply Chain Technology

  3. Jeff Williams says:

    Ninja – you might want to take a look at the OWASP Application Security Verification Standard (ASVS). It details all the requirements for verifying the security of an application regardless of the technique you use to do it. Choosing just the requirements you can verify with code review makes it difficult to understand the assurance you’re actually getting.

  4. Nilesh says:

    Hi,

    Excellent work!! waiting for more checklists. Really it’s helpful. Thanks!

  5. Security Ninja says:

    Hi Jeff,

    Thanks for comment. I’ve very briefly looked at the ASVS and spoken with a few of the project contributors to try and get a better understanding of it. I will have a better read of it over the weekend.

    SN

  6. Stephan Wehner says:

    I find your items are worded quite technically. But most are about input validation. Then you have two about encoding.

    I thought a list should look something like this.

    * Pitfalls of arithmetic: overflow, rounding errors, division by zero.
    * Application of the proper time zone.
    * Unit tests for all code paths.
    * etc.

    (I don’t have a full list)

    Cheers

    Stephan

  7. Security Ninja says:

    Hi Stephan,

    Thanks for commenting on the blog post. The checklist items this week were only for Input and Output Validation, we will be posting more over the coming weeks covering other secure development principles such as Error Handling and Secure Communications.

    I like the points you listed but I don’t see them as items on a security persons code review checklist.

    Thanks,

    SN

  8. Pingback: Week 50 in Review – 2009 | Infosec Events

  9. iVictor says:

    Hi SN,

    Good post. Just a quick thought.

    Although you have put it across in a simple & it’s quite easy to understand, some points could certainly do with sample short n dirty examples.

    Thanks for this post. Reading on to parts II & III.

    Best Regards.

  10. Security Ninja says:

    Hi iVictor,

    Thanks for stopping by and leaving a comment. I can see what you are saying, I think I have just the thing for this. We have developed a full application internally which is used for training purposes so I can probably use code from that to demonstrate some of the checklist items.

    Did you have any specific items that you think could have been better explained by using a code sample?

    SN

  11. Pingback: Interesting Information Security Bits for 01/05/2010 | Infosec Ramblings

  12. Pingback: un-excogitate.org » Blog Archiv » 2010 Focus

  13. Dragon says:

    Hi Security Ninja,

    Really informative post. Though not a developer, I still find it simple to follow.

    I have a question. how would we perform validation on HTTP request headers? If you could give some example code for showing this, it’d be really helpful.

    Thanks.
    Dragon

  14. Pingback: Checklist for security code reviews by Security Ninja | Yuji Kosuga's Blog

  15. B says:

    This is good, but guess what folks? Not all apps are webapps. Some are client server, in which case most of the checklist items aren’t in scope.

  16. Security Ninja says:

    I agree, what checklist items are you going to propose then so we can expand the checklist?

    SN

Leave a comment

VIDEOS & SLIDESHARES

Look at our latest security Videos & SlideShares

EVENTS & SEMINARS

Upcoming Security Events & Seminars

PODCASTS & DOWNLOADS

Check out our Podcasts & White Papers