How to Catch Injection Security Vulnerabilities in Code Review

Understanding Injection Vulnerabilities

Injection vulnerabilities exist when information provided by users of the application is not properly validated or sanitized before it is used. In the case of Cross-Site Scripting, this would mean that a field allows the entry of JavaScript which is then re-displayed to the client. For SQL injection, this means a field that is written to the database is not properly validated and a SQL sanitization function is not run against the data before it is sent to the database as part of a command for execution. The last vulnerability they enumerate is “External Control of File Name or Path” this means that a field allows the user to insert a file name or a path that is used to write or read from the underlying operating system on the server. In each case, we have missing data validation. So, for our working definition let’s go with this:

Injection vulnerabilities are missing validations and sanitizations of user input that allow a user to gain access to more data than they should.

Catching Missing Validations and Sanitizations in Code Review

Missing validation and sanitization of inputs are not simple to find through automated testing, and they are not consistently found through click testing of applications before deployment. Most testers are not sufficiently trained in attack vectors to find these problems, and many are hidden away in APIs or may be obscured by client-side data validation. Once you know what to look for, though, these vulnerabilities jump off the screen in a code review.

Catching JavaScript Injection Vulnerabilities

JavaScript injection vulnerabilities, the relevant component of Cross-Site Scripting, happen most frequently when a text input field is written to the database and then displayed later without any sanitization of the displayed information. So, what sanitization do we need here? Primarily we need to turn certain special characters into their entity-encoding before display.

So, if someone inputs this into a text box:

When we write it back out to the page we want to turn it into this:

The greater-than and less-than symbols have been turned into their HTML entity equivalents, so this will not be interpreted as HTML and the JavaScript function enclosed won’t be executed.

Different languages have different methods for escaping HTML. Reading up on these methods will help prepare you. In Ruby, I know to look for the following telltale signs:

• Using the .html_safe function in Rails views.
• Not using CGI::escapeHTML outside-of-view contexts with auto-escaping.
• Using raw markdown compiled without protecting against JavaScript tag injection.

Depending on the language, you’ll find many different telltale signs of JavaScript injection vulnerabilities that can result in Cross-Site Scripting. Become familiar with the HTML escaping system in the language you’re using day-to-day, and you’ll find your own list to look for.

Catching SQL Injection Vulnerabilities

SQL injection vulnerabilities occur when strings from the client are inserted directly into a SQL statement without first being sanitized. The most common way that this happens is string interpolation.

In Ruby, for example, the following code is insecure:

This is because the ID parameter can be replaced with any string the user desires and it will be inserted directly into the SQL statement without any escaping. The secure version would leverage the ORM to perform the same query:

Using ORMs correctly will generally remove any risk of SQL injection, though the proper approach differs from ORM to ORM. In ActiveRecord, there are methods that take additional arguments that allow information to be safely inserted into SQL strings.

For example:

The question mark syntax indicates to the ORM where to insert the escaped version of the second parameter. Most other ORMs will provide similar functionality but it’s good practice to understand how much sanitization is taken care of at the ORM level and the ORM’s limits in handling sanitization.

The biggest red flag to look for in code review is string interpolation being used in SQL queries. This is generally unsafe.

Catching External Control of File Paths

External control of file paths is something that crops up when users are allowed to upload a file or initialize a resource server-side, such as a SQLite database. It can also happen on reading if a system is not configured properly to only read from certain locations.

The danger in allowing the user to control the path is that they can attempt to write files to dangerous places, or gain access to the contents of source code or other files that contain privileged information.

Catching this type of security issue requires that the programmer or reviewer be aware of instances where a file path can be inserted into a parameter—this could be something straightforward like a file upload path or file read command, or somewhere slightly more unexpected such as a database connection string.

I look for instances where this information is passed in as a full path rather than as a sub-path of a containing folder or the more secure route of saving off the intended path to the database but writing the file to a random location on disk to avoid possible security issues.

Conclusion

Injection vulnerabilities result from insecure handling of user inputs. They are relatively simple to fix once the underlying issues that cause them are understood, and are frequently found by experienced reviewers who know what to look for. The prevalence of injection vulnerabilities today is one of the best arguments for continuing to perform code review in many organizations—this type of vulnerability is most frequently caught through human inspection of the offending code.

This post was originally published on the PullRequest website. On April 28th, 2022 HackerOne acquired PullRequest to help power developer-first security testing solutions. 

Find post author Will Barrett here.