/jan 2, 2024

Using Veracode Fix to Remediate an SQL Injection Flaw

By Robert Haynes

Introduction 

In this first in a series of articles looking at how to remediate common flaws using Veracode Fix – Veracode’s AI security remediation assistant, we will look at finding and fixing one of the most common and persistent flaw types – an SQL injection attack.

An SQL injection attack is a malicious exploit where an attacker injects unauthorized SQL code into input fields of a web application, aiming to manipulate the application's database. By manipulating input parameters, attackers can trick the application into executing unintended SQL commands. This can lead to unauthorized access, data retrieval, modification, or even deletion. Successful SQL injection attacks compromise data integrity and confidentiality, posing serious security risks.

Example Code and Analysis

Let’s look at a weakness in the source code of the deliberately vulnerable (and freely available) Verademo application, specifically the UserController.java source file found in the application repository in   ./app/src/main/java/com/veracode/verademo/controller/UserController.java.

Since this is a deliberately vulnerable app, the writers have handily commented the vulnerable code:

/* START EXAMPLE VULNERABILITY */

// Execute the query                 

logger.info("Creating the Statement");

String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username='" + username + "' and password='" +    md5(password) + "';";

sqlStatement = connect.createStatement();      

logger.info("Execute the Statement");

ResultSet result = sqlStatement.executeQuery(sqlQuery);

/* END EXAMPLE VULNERABILITY */

Here you can see that the code builds a SQL query by concatenating a base ‘SELECT’ statement with variables username and an MD5 sum of password . This query is then executed with no further checking. If an attacker can control the value of the username variable, then they can inject commands into the database.

Let’s say I enter something malicious as my username which sets the username variable:

John’” and password=foo’"; DROP TABLE users_history; --

The executed SQL query will be:

select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username='"John’"; DROP TABLE users_history; -- "' and password='" + md5(password) + "';"      

Dropping the users_history table might be a little inconvenient. Of course, an attacker may need to try multiple strings until an effective exploit is found, but the danger is clear.

Running a Static Analysis Scan

Veracode Static Analysis is a powerful and accurate static analysis SaaS service that detects, classifies, and prioritizes security flaws in application code.  Let’s see what Veracode Static Analysis says when we run it against this project.

First, we will need to package the code for analysis (assuming it has been compiled already). In this case, we’re using the (currently experimental) ‘package’ subcommand for the Veracode CLI:

veracode package --source verademo/app --output verademo/app                           

STATIC-PACK INFO: Copying Java artifacts for Maven project.

STATIC-PACK INFO: Copied artifact: verademo/app/verademo.war.

STATIC-PACK INFO: Successfully created 1 artifact(s).

STATIC-PACK INFO: Created Java artifacts for Maven project.

STATIC-PACK INFO: Total time taken to package artifacts: 10.545s

Next, we can use Veracode static scan to upload and scan the application and put the results into a machine-readable json file:

veracode static scan verademo/app/verademo.war --results-file verademo/app/results.json

Since in this example, we’re only interested in UserController.java, we will skip looking at all the results and just run Veracode Fix against our file, and pick the issue that corresponds to the code above. 

Remediating with Veracode Fix

Veracode Fix ‘closes the loop’ between flaws discovered by Veracode SAST by providing AI-generated remediation suggestions that can be applied directly to your code. Veracode Fix adapts ‘reference patches’ created by Veracode security experts to work with your code. This makes fixing common application security flaws quick and easy.

veracode fix verademo/app/src/main/java/com/veracode/verademo/controller/UserController.java --results verademo/app/results.json

Veracode will list all the discovered CWE's in the source and ask us which one to address:

----------------------------------------------------------------

Veracode Fix

Version 2.5.1

Issues in source file UserController.java:

    1: CWEId 89

                 Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') on line 165 in function com.veracode.verademo.controller.UserController.processLogin

…

<many more problems>

…

Enter issue number:

Looking at our example above, we’re interested in issue number 1 as it corresponds to line 165 in the code where our deliberately vulnerable SQL is executed.

Sometimes Fix offers several ways to remediate the problem, but in this case has found only one acceptable solution (the Fix model only shows solutions that meet confidence levels for both fixing the problem and ensuring the code compiles). 

The solutions are displayed in a Git style diff to illustrate the changes:

--- FIX 1 -------------------------------------------------------------

--- verademo/app/src/main/java/com/veracode/verademo/controller/UserController.java
+++ verademo/app/src/main/java/com/veracode/verademo/controller/UserController.java
@@ -147,7 +147,7 @@
 		}
 
 		Connection connect = null;
-		Statement sqlStatement = null;
+		PreparedStatement sqlStatement = null;
 
 		try {
 			// Get the Database Connection
@@ -158,11 +158,13 @@
 			/* START EXAMPLE VULNERABILITY */
 			// Execute the query
 			logger.info("Creating the Statement");
-			String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username='"
-					+ username + "' and password='" + md5(password) + "';";
-			sqlStatement = connect.createStatement();
+			String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username=? and password=?;";
 			logger.info("Execute the Statement");
-			ResultSet result = sqlStatement.executeQuery(sqlQuery);
+			sqlStatement = connect.prepareStatement(sqlQuery);
+		sqlStatement.setString(1, username);
+		sqlStatement.setString(2, md5(password));
+
+			ResultSet result = sqlStatement.executeQuery();
 			/* END EXAMPLE VULNERABILITY */
 
 			// Did we find exactly 1 user that matched?

Enter the fix to apply. [1-1]: 1

Choosing option 1 results in this new code:

/* START EXAMPLE VULNERABILITY */

// Execute the query

logger.info("Creating the Statement");

String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username=? and password=?;";

logger.info("Execute the Statement");

sqlStatement = connect.prepareStatement(sqlQuery);

sqlStatement.setString(1, username);

sqlStatement.setString(2, md5(password));

ResultSet result = sqlStatement.executeQuery();

/* END EXAMPLE VULNERABILITY */

So what does the new code do? You can see that instead of simply constructing a string and using that as the SQL query to execute, we are using the PreparedStatement function to use a parameterized query, then supplying the variables as parameters to the query before executing the prepared statement.

New parameterized query string:

String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username=? and password=?;";

Prepared statement:

PreparedStatement sqlStatement = connect.prepareStatement(sqlQuery);

Binding the variables as parameters:

sqlStatement.setString(1, username);

sqlStatement.setString(2, md5(password));

Executing the statement:

ResultSet result = sqlStatement.executeQuery();

Using a prepared statement helps prevent SQL injection attacks in several ways:

Parameter Binding to Treat Input as Data Only

Prepared statements separate SQL code from data by using placeholders for parameters. These placeholders are then bound to actual values, ensuring user input is treated as data, not executable code. This eliminates the risk of SQL injection since user input doesn't directly affect the structure of the SQL query.

Automatic Escaping to Sanitize Special Characters

When using prepared statements, the database driver automatically handles proper escaping and quoting of parameter values. This ensures that special characters within the input data are treated as literal values, preventing them from being interpreted as part of the SQL code.

Prevention of Code Injection through Query Manipulation

With prepared statements, the structure of the SQL query remains constant, and placeholders are used for data. This makes it nearly impossible for an attacker to inject malicious SQL code because they cannot alter the structure of the query itself.

Of course, we should now recompile our code, repackage, and retest it to ensure that we have fixed the problem (spoiler: it compiles and the SAST scan no longer flags this CWE).

Conclusion

In this example, we have shown how Veracode Fix uses a prepared statement to remediate a SQL Injection flaw discovered by Veracode Static Analysis, and hopefully given you the knowledge to avoid coding insecure SQL queries in the future. Of course, for the code you’ve inherited Veracode Static Analysis and Veracode Fix are here to help you fix these flaws before it’s too late.

Learn more about Veracode SAST, Veracode Fix, and developer training, or get in contact to learn more about Veracode’s security tools and services.

Related Posts

By Robert Haynes

Robert’s quarter-century working in IT has progressed (or is that regressed?) through helpdesk, UNIX sysadmin, backup, storage, application security,  technical sales, and marketing.  He now spends his time hanging out at the intersection of artificial intelligence and human ingenuity, waving a sign that says: “This way for secure software."