10-point secure code review checklist for developers

Security should always be “by design.”

And secure code reviews ensure that the implementation of that design is foundationally correct. 

They allow apps to align with specific security requirements without compromising functionality and hindering development. As such, code reviews fit into an overall strategy of coding practices that follow a secure development lifecycle. 

But how do you encourage development teams to embrace secure coding practices? 

This article will focus on conducting code reviews to check for security issues in the code. We’ll focus on how you can build security-focused systems and workflows that make code reviews easier to adopt, implement, and sustain. 

Preparing for secure code reviews

Before submitting code to peers, developers should ensure their work is ready for a code review. Reviewers must be able to understand why the code was written and whether the submitted code satisfies those requirements. 

Code review preparation checklists are useful here. They help developers communicate the:  

Size of the code change

The code under review should be relatively small and confined to dealing with a specific issue. This might mean splitting the code into several pull requests (PR) and reviewing them independently. 

Views differ on what a “small” amount of code is. But a good rule of thumb is that the code reviewed should be less than 500 lines of code.

Changes to be documented

The purpose of the code should be documented and how the code addressed the requirements. In repositories like GitHub, pull request templates can be used to ensure that the required information is provided. 

Tests to be conducted

Code submissions introduce new tests, which must pass successfully. It's crucial to verify that these additions do not interfere with or fail any existing tests within an automated testing framework. 

Developers are responsible for confirming their updates do not disrupt the overall build's integrity.

When you're doing code reviews, remember they're not just about the code itself. Like, if you're looking at an API, there's a good chance it's got a reverse proxy or something like that in front of it. It's about looking beyond just the code review and considering the environment around the code too.

 

Vitor Costa (bus actor), Senior Customer Support, Hack The Box

Code formatting and linting

The developer should have checked that the code conforms to the organization’s style guide and that linting checks return no warnings or errors.

Static analysis and security testing results

If available, a static analysis and security testing tool (OWASP’s website has a list of suitable tools) should be used to highlight security issues with the code. These should be remediated before the commit but if not, developers  should leave a note as to why  issues haven’t been addressed.

Good developers will have enough experience to understand what they should be doing when it comes to organizing their code and avoiding common security vulnerabilities. 

However, anyone can make errors. So the purpose of the code review is to focus on specific areas to look for potential security vulnerabilities.

A note on “shifting left” for reviews and testing: 

Traditional secure code review processes, when implemented late in the cycle, can significantly slow down development.  What happens if you find an issue?

 

Do you ignore the risk and deploy that to production? Or do you ask devs to go back and fix the issues, potentially delaying the project timeline?

 

Reviews and testing too late in the pipeline lead to cumbersome cycles of identifying issues, contacting developers for fixes, and undergoing multiple rounds of testing, code review, and builds. 

 

This not only puts pressure on developers but also increases the risk of rushing fixes to meet deadlines, potentially compromising the security of the production environment.

 

By testing early on such as directly within the Integrated Development Environment (IDE) and within the Pull Requests (PR) with the right tools and processes, developers can address security concerns as they code. 

 

This "shift-left" approach is something we champion at Snyk because it improves secure coding practices for both devs and security professionals.

Billy Yung, Staff Solutions Engineer, Snyk

10-point checklist for code security flaws

With code now submitted for review, it’s time for a peer to begin assessing it using the documentation or notes provided. But what common or critical security issues should developers look for?

The checklist below breaks down different issues that make code vulnerable and lists specifics to watch out for.

1. Input validation issues

What to look for: 

• Inputs from external sources are validated.

• User input is tested for type, length, format, and range, and by enforcing limits.

• Regular expressions are checked for flaws that could cause data validation problems.

• Check that input is validated using: Exact match approaches and/or, Allow lists and/or, Block lists

• XML documents should be validated against their schemas.

• String concatenations should not be used for user input.

• SQL statements should not be dynamically created by using user input.

• Data should always be (re)-validated on the server side.

• Check that there is a strong separation between data and commands, and data and client-side scripts.

• Check for contextual escaping use when passing data to SQL, LDAP, OS, and third-party commands.

• Validate https headers for each request.

Security mapping: 

This maps to vulnerabilities like SQL injection, Cross-site Scripting (XSS), and command injection, where malicious input can be used to exploit the system.

7. Cross-site request forgery (CSRF) protections

What to look for: 

• Check for CSRF tokens in applications that perform state-changing operations based on user requests. Many development frameworks (like Django for example) include simple means of enabling CSRF protection in applications. OWASP provides a more detailed look at how to check for CSRF vulnerabilities.

Security mapping: 

Lack of CSRF protections can lead to unauthorized actions on behalf of the authenticated user.

8. Server-side code execution validation

Code should never execute directly from user input. 

There are multiple ways of carrying out actions based on user commands that are controlled and safe. So generally, if this has been implemented, the question should be how to turn this function into a more controlled form of action selection.

What to look for: 

• Ensure that server-side code properly validates user input and does not execute untrusted data.

Security mapping: 

This is crucial to prevent Remote Code Execution vulnerabilities, where attackers can run arbitrary code on the server.

9. Business logic errors

What to look for: 

• Identify flaws in the application logic that could be exploited, such as bypassing payment processes or changing user privilege levels.

Security mapping: 

These errors can lead to Business Logic Vulnerabilities, which can be devastating as they often circumvent traditional security measures.

10. Code quality and best practices

What to look for: 

• The developer has submitted all required documentation, test results, and analysis scans.

• The code addresses the reasons for it being committed to the code base.

• Code adheres to the team’s style guide.

• Code is properly documented such that reviewers can understand its purpose and structure.

Security mapping: 

While not directly a security issue, poor code quality can lead to vulnerabilities being introduced inadvertently and make security auditing more difficult.

Any issues found should be documented and a suggestion made as to how to resolve it. The developer will then agree to the change and on subsequent updates of the code, the reviewer can check that the issue was resolved.

Although not mentioned specifically, metrics can be collected and analyzed to track improvements in overall quality. These metrics can highlight problem areas that may need attention and adjustment. 

Although the use of metrics is desirable, they are less important than embedding the code reviews into everyday development practice.

Measuring the impact of secure coding 

Honoring secure practices without harming functionality or implementation

As with all process improvement, code reviews should be a natural step in the pipeline of developing and maintaining software. Most importantly, it should be done within a positive culture where the review is seen as a learning experience that ultimately increases the quality of the code and the skills of the developer and reviewers. 

The use of templates, automation, and checklists helps reduce the overhead of code reviews but generally, they only add a small increment of time to the overall process—and provide a huge ROI in terms of long-term security and functionality.