Eliminating a Class of Defects

Imagine the following line of PHP code:

$db->GetRow("SELECT * FROM users WHERE id = $user_id");

This line is vulnerable to SQL injection, if an attacker controls $user_id variable.

This is the safe version (using parametrized query):

$db->GetRow("SELECT * FROM users WHERE id = ?", array(‘$user_id’));

Options

You have three options to deal with this problem:

Option #1: Before each release, you manually check for SQL injections in your applications. You insert single quote in text fields and look for errors. This is tedious work and you may get bored pretty quickly. You can use a bunch of tools to assist you - like SQLMap or Netsparker. The automated scanner will save some time but will not uncover all problems. The tools that test through the UI cannot reach certain pages that are buried three levels deep. Sometimes the authentication fails, sometimes your application needs to be configures in certain way to produce meaningful results for the scanners. You make a checkbox in your pre-release checklist to scan for SQL injections before every release. This option is also the slowest of the three. I’ve worked at a companies, where extensive SQL injections scan with automatic tool takes 8-10 hours.

Option #2: To save manual work before every release, you can write a unit test for the method that contains the line above. Although the unit test will not call the DB (it runs only in memory), you can verify that the GetRow method is called with two arguments instead of one. You can also check that the first argument is a string that equals to “SELECT * FROM users WHERE id = ?”. Job done, no? The problem remains. The unit test covers only one vulnerable location in your code base. Writing unit tests to check specifically for SQL injection is overkill and it couples tightly code and tests. If you change the table name or the SQL query parameters, you need to also change the test.

Side note: if you decide to go with this option, it is better to use high level automated tests (API or UI), rather than unit.

Option #3: You decide that option #1 and option #2 are unreliable and overkill. You bite the bullet and start writing a tool that scans all of your backend code for possible SQL injections vulnerabilities. You want to remove them once and for all. At Komfo, we did exactly this and developed a tool to remove a whole class of software and security defects from our code. We never have to worry about this types of errors. Whenever you can, you should pursue this option.

The table bellow summarizes the three approaches:

Our tool was working so good that we decided to create another one and eliminate а different class of defects — unintended table locks. This tool is called PHP-Unlocker.

Custom Tool Advantages

Although the initial price to develop such a tool is high, the ongoing maintenance effort is low or nonexistent. You’ll setup the tool once and it does not need constant adjustments. Also static code analysis tools are lightning fast and can be run in your CI pipeline after every code change. They analyze 100% of your code base (a unit test will analyze only the method that its written for). You can either choose to write a tool from scratch, the way we did with PHP-Reaper and PHP-Unlocker ,or (depending on your language of choice) can write plugin for popular static code analysis tool — e.g. FindBugs for Java.

Your custom tool does not have to be static code analyzer, because the class of bugs you need to eradicate may be different in nature. It can be a shell script run at the end of test environment setup to check the logs for errors. Or it can be an nmap scan of your application for open ports after all automated tests are completed.

The Three Pillars of Automated Testing

A while ago I wrote a blog post about the three pillars of automated testing — static code analysis, unit tests and high level tests. Do not ignore the first of those pillars, it pays off. Static code analysis tools offer tremendous benefits in some cases. Learn how to use it. As a rule of thumb you should push the tests as close to the code as possible (high level tests -> unit tests -> static code analysis -> code). The higher you go, the more expensive the tests become (time, effort, reliability and maintenance).

Your End Goal

Strive to remove a whole class of defects instead of a single one. Whenever you encounter a problem, don’t eliminate it quickly on the spot. Think of how you can detect and remove the whole class/type of similar defects when they occur in the future. What is their common denominator? What is the root cause? Then you automate the process by writing a tool and running it on every code change. Time to move on to more creative work. Don't fight the same battles constantly.