Skip to main content

What to Look for in a Security Code Review

Overview

The following sections contain checklist style questions for each major focus area. Use these questions to drive your code reviews based on which are applicable to the code under review.

Not every item in this checklist needs to be applied to every code review, in fact for most code reviews it’ll be a small subset. When first applying this checklist to your code review process, you may not immediately know which sections are relevant. Over time this will become easier as you become more experienced with the process.

Common Vulnerabilities

Command Injection

  • Does the code call into a system shell or other command interpreter?

  • Is it possible for a user (through forms, cookies, headers, etc) to influence parameters or commands passed to the shell?

  • If users can influence the shell, is the input allow-list validated and/or only safe shell commands used?

  • Is appropriate escaping or encoding used?

  • Likewise, if the code passes to a command line, can a user influence the command line call?

Cross-Site Request Forgery

  • Is there a unique token (or some other form of CSRF protection) in place on every HTTP request that executes business logic?

  • Is bearer token or other HTTP header-based token added automatically by the browser? (If added manually by code on the page, CSRF is much harder to accomplish)

  • Does application silently fall back to cookie validation if a bearer token is absent? (This can subvert protection from above)

Cross-Site Scripting (XSS)

  • Is all user input encoded or sanitized according to the context in which it will be used?

CSS Injection

  • Is user input (or other untrusted input) echoed into CSS?

  • If so, is it limited to property value and CSS encoded?

Expression Language Injection

  • Is an EL interpreter used?

  • Is user data passed to the interpreter without validation or encoding?

File Upload/Download Issues

  • Can a user influence path for a file upload or download?

  • Are file uploads restricted to certain file types and fully validated by more than just file extension?

  • Are ACLs used to lock down file paths that an upload can end up in?

  • Could the file upload functionality be used to execute a DoS attack?

  • Are file names properly treated as user input and validated prior to storage in the database?

  • Are file names encoded before being echoed back to the user?

HTTP Request Smuggling

  • Is HTTP/2 used for all backend connections?

  • Are all web servers using the same type of HTTP header processing?

HTTP Verb Processing

  • Make sure there are no security controls that rely on HTTP verbs.

  • If locking down what verbs are allowed, is an allow list used?

Injection Issues

  • Is user input used to construct a database query?

  • Are all SQL and NoSQL queries parameterized?

  • Is ORM used for all relational database access?

  • Is user input stored in log files that are later processed by any other part of the app as input? This could result in second-order injection attacks. 

  • Is user input (or other untrusted input) used to build JavaScript queries into a database?

  • Is JSON used as a parameter to eval()?

  • Is JSON used to construct dynamic HTML?

  • Is CSP policy understood and used appropriately?

  • Are HTTP headers encoded before being echoed back to the page?

Insecure Direct Object Reference

  • Are request parameters checked to make sure they are valid for the user?

  • Is user input used in any other way to reference an object? If so, is it checked to ensure it is valid for the user?

  • Are all requests for objects authorized against user permissions on every request?

  • Is there data binding that would allow a user to modify a hidden parameter?

Server Side Template Injection

  • Does the code rely on a template engine?

  • If so, is it an engine without any known vulnerabilities?

  • Is it an engine with logic? If so, can it be replaced by one that is logic-less?

  • Is untrusted input passed to the engine, or used to modify templates?

Tabnabbing

  • Are you using rel="noopener noreferrer" attribute for all HTML links?

  • Are you using noopener and noreferrer on the windowFeatures parameter for window.open?

Unvalidated Redirects

  • Does the application allow a redirect based upon a URL parameter?

  • Is a forward executed in code based on a URL parameter?

  • Are there any other ways for user input to influence a redirect or forward?

  • Are all redirects or forward either checked against an allow-list, kept to relative paths, or based on a lookup table to requires an authenticated user to edit?

URL Parameter Manipulation

  • Are URL params used to perform any sensitive operations?

  • Is data returned to the user based on a predictable URL param lookup?

XML External Entity Injection

  • Are XML parsers used?

  • If so, is doctype processing disabled?

  • Are all frameworks and libraries up to date, in order to gain the latest, most secure parsers?

  • Whenever external entities are permitted, is there an allow-list for remote entities or hosts which are allowed to be referenced?

Authentication

  • Is login only over TLS

  • Are passwords (valid or invalid) kept out of logs?

  • Can tokens and credentials be expired?

  • Are credentials or tokens stored in clear text?

  • Are authentication mechanisms rate limited?

  • Are session identifiers encrypted?

  • Do sessions expire quickly enough?

  • Are sessions cryptographically random?

  • Are session cookies HTTP-Only?

  • Are only session IDs generated by the application accepted?

  • Are authentication APIs rate limited?

  • If we store a password, is it absolutely necessary to store the password or could we use another authentication system such as Okta or Cognito?

  • If it is necessary to store a password, is it only stored as a cryptographically salted hash?

Authorization

  • Are permissions checked every time a request is made?

  • Do permissions follow least privilege?

  • Is user input used to directly reference business logic?

  • Is authorization dependent upon untrusted user-supplied data?

  • Is deny-by-default logic used?

  • Is execution flow correct in failure cases?

  • Is access control in place for all resources?

  • Are all authorization checks done on the server side and not the client?

  • Are sessions handled correctly?

  • What functionality can be accessed without authentication?

  • Are all admin interfaces protected behind Okta and/or Trident?

Configuration

  • Are permissions configured for least privilege?

  • Are there any unnecessary features, ports, or other aspects of attack surface that can be turned off?

  • Are all components and libraries updated to latest, most secure versions?

  • Is Strict-Transport-Security turned on?

  • Is Content-Security-Policy turned on?

CORs

  • Has CORS been specified so that only trusted domains are allowed (allow-list)?

Data Access

  • Are connections to external servers secure?

  • Are inputs from external servers validated?

Deserialization

  • Does the code deserialize data (RPC/IPC, wire protocols, web services, message brokers, caching, db, file system, HTTP cookies, form params, authentication tokens)?

  • Is the serialized data trustworthy or has it crossed a trust boundary at some point without validation?

  • Is a signature used to validate integrity and authenticity?

  • If unvalidated data, does this code perform data validation for range, format, type, and length?

Error Handling

  • Do error messages give away any internal system or state information that could help an attacker?

  • Does the application display only generic error messages to the user, without identifying underlying business logic, frameworks, or versions of software?

  • Are error messages using client-side validation backed up by server-side validation?

  • Does the code fail securely?

  • Are there any error conditions that indicate malicious activity that should be logged or alerted on?

  • Does the difference in response time between success and error make it easy for a caller to determine success or failure based only on time taken? (Consider other back-channel indicators (beyond timing) that could help an attacker as well)

Hard Coded Secrets

  • Is there sensitive data in configuration files?

  • Is there sensitive data in the code?

Input and Data Validation

  • Is each external source of data validated for range, type, format, and length?

    • Browser input including http headers, form parameters, and URL parameters

    • API input

    • Cookies

    • External processes

    • Data feeds

    • Service responses

    • File input

    • Command line parameters

    • Environment variables

  • Is input and data validation performed on the server?

Logging

  • Is any sensitive data making it into the logs?

Sensitive Data and Crypto

  • Is HTTPS required everywhere?

  • Is TLS 1.2 or higher used?

  • Is Perfect Forward Secrecy cipher suites prioritized in configuration?

  • Are Cipher Block Chaining CBC encryption modes low priority in configuration?

  • Is HTTP Strict Transport Security (HSTS) enabled?

  • Is secure flag used on cookies?

  • Is http-only flag used on cookies?

  • Is sensitive data exposed in the URL?

  • Is sensitive data cached in clear text on proxies or on the client?

  • Is platform supported cryptography used rather than a custom implementation?

  • Are weak cipher suites disabled (DES, 3DES, RC2, RC4, and Null)?

  • Is DHE key exchange disabled with RSA?

  • When making changes to an endpoint without an authorization check, make sure secrets or sensitive data are not being exposed.

Sessions

  • Are sessions expired?

  • Are session IDs unpredictable?

  • Is all session information protected and encrypted in transit?

In this section: