How to fix CWE 89 SQL Injection flaws?

General remediation tip:
Avoid dynamically constructing SQL queries. Instead, use parameterized prepared statements to prevent the database from interpreting the contents of bind variables as part of the query. Always validate untrusted input to ensure that it conforms to the expected format, using centralized data validation routines when possible.
Following mitigation strategies can be combined to severely limit the SQL Injection exploits.

Mitigation Strategy 1] Using Prepared Statement.

Both Statement and PreparedStatement can be used to execute SQL queries. These interfaces look very similar. However, they differ significantly from one another in features and performance
• Statement – Used to execute string-based SQL queries
• PreparedStatement – Used to execute parameterized SQL queries
Let us continue the explanation with a help of few problematic codes:
We are using Java example for explanation and the approach can be adapted in other technologies as well.
Example 1:

Public void search (String employeeId) {
try{
Statement stmt = connection.createStatement();
ResultSet  resultSet = null;    
resultSet = Stmt.executeQuery(“SELECT * FROM EmployeeTbl WHERE id=’”+employeeId+”’”);
}catch(Exception e) { }
}

Example 2:

public void insert(Employee employee) { 
String query = "INSERT INTO EmployeeTbl(id, name, email) VALUES(" + employee.getId() + ", '" + employee.getName() + "', '" + employee.getEmail() + "')"; Statement statement = connection.createStatement(); 
statement.executeUpdate(query); 
}

Example 3:

public void update(Employee employee) { 
String query = "UPDATE EmployeeTbl SET name = '" + employee.getName() + "', email = '" + employee.getEmail() + "' WHERE id = "+employee.getId()+")"; 
Statement statement = connection.createStatement(); 
statement.executeUpdate(query); 
}

Example 4:

public void delete(Employee employee) { 
String query = "DELETE FROM EmployeeTbl WHERE id = "+employee.getId(); 
Statement statement = connection.createStatement(); 
statement.executeUpdate(query); 
}

The Statement accepts SQL query as String, and we see that numerous variables are used in the dynamic construction of the SQL query using String concatenation making it not only Vulnerable to SQL injection but also making it less readable.
Why the above examples are vulnerable?

Example 1 receives employeeId which ideally is just an alphanumeric string. Assume the variable employeeId is “A1001”.
The final SQL query will be :

SELECT * FROM EmployeeTbl WHERE id=’A1001’

This is fine, but if this variable is manipulated by an attacker, for e.g. “A1001’ or ‘1’=’1”. The final SQL query that would be executed will be :

SELECT * FROM EmployeeTbl WHERE id=’A1001’ or ‘1’=’1’

That is nullifying the condition of the Employee Id check.
This is a classic example of SQL injection.

Example 2:
Assume the value of name and email is “Derek O’ Brien” and “derek@google.com” respectively. That would result in executing of :

INSERT INTO EmployeeTbl(id, name, email) VALUES(2001,’Derek O’Brien’,’derek@google.com’)

This would fail to insert the record in the database due to an extra single quote in the name column.

Example 3:
Assume the attacker modifies the value for email. “sarah@google.com’ where id =101 --“ which would make the final SQL query as :

UPDATE EmployeeTbl SET name = 'Sarah', email = 'sarah@google.com’ where id =101 --' WHERE id = 2001

That would update a different record than the intended one. ( Id=101 instead of id=2001 because “- -“ is interpreted as a comment in SQL)

Example 4:
Assume the value for id is “2001 or ‘1’=’1’”, which would make the final SQL query as :

DELETE FROM EmployeeTbl WHERE id =2001 or ‘1’=’1’

That would delete all records of the table.

Other points to take note of regarding the Statement.
a) JDBC passes the query with inline values to the database. Therefore, there's no query optimization, and most importantly, the database engine must ensure all the checks. Also, the query will not appear as the same to the database and it will prevent cache usage.
b) Batch updates need to be executed separately:

public void insertAllEmployees(List<Employee> employeeList) {
    for (Employee empl: employeeList) {
        insert(empl); //This would execute individual INSERT queries.
    }
}

c) The Statement interface is suitable for DDL queries like CREATE, ALTER, and DROP:

public void createTables() {
    String query = "create table if not exists PERSONS (ID INT, NAME VARCHAR(45))";
    connection.createStatement().executeUpdate(query);
}

d) Also note that the Statement interface can’t be used for storing and retrieving files and arrays.
PreparedStatement extends the Statement interface and it has methods that help bind various object types.
For e.g. Example 1 can be rewritten as:

Public void search (String employeeId) {
try{
PreparedStatement pstmt = connection.prepareStatement((“SELECT * FROM EmployeeTbl WHERE id=’?’”);
ResultSet  resultSet = null;    
Pstmt.setString(1,employeeId); //replacing the ‘?’ with actual value
resultSet = pstmt.executeQuery();
}catch(Exception e) { }
}

The code becomes more readable and easy to understand.
It protects against SQL Injection, by escaping the text for all the parameter values provided.

Example 2:

public void insert(Employee employee) { 
String query = "INSERT INTO EmployeeTbl(id, name, email) VALUES(’?’,’?’,’?’)"; 
PreparedStatement pstatement = connection.prepareStatement(query); 
Pstatement.setString(1,employee.getId());
Pstatement.setString(2,employee.getName());
Pstatement.setString(3,employee.getEmail());
pstatement.executeUpdate(); 
}

Example 3:

public void update(Employee employee) { 
String query = "UPDATE EmployeeTbl SET name = '?', email = '?' WHERE id =’?’)"; 
PreparedStatement pstatement = connection.prepareStatement(query); 
Pstatement.setString(1,employee.getName());
Pstatement.setString(2,employee.getEmail());
Pstatement.setString(3,employee.getId());
pstatement.executeUpdate(); 
}

Example 4:

public void delete(Employee employee) { 
String query = "DELETE FROM EmployeeTbl WHERE id = ?"; 
PreparedStatement pstatement = connection.prepareStatement(query); 
Pstatement.setString(1,employee.getId());
pstatement.executeUpdate(); 
}

The PreparedStatement uses pre-compilation. As soon as the database gets a query, it will check the cache before pre-compiling the query. Consequently, if it is not cached, the database engine will save it for the next usage.
The PreparedStatement provides a batch execution during a single database connection.
Example:

public void insertAllEmployees (List<Employee> employeeList) {
    String query = "INSERT INTO employee(id, name) VALUES( ?, ?)";
    PreparedStatement preparedStatement = connection.prepareStatement(query);
    for (Employee employee: employeeList) {
        preparedStatement.setInt(1, employee.getId());
        preparedStatement.setString(2, employee.getName());
        preparedStatement.addBatch();
    }
    preparedStatement.executeBatch();
 
}

PreparedStatement provides an easy way to store and retrieve files by using BLOB and CLOB data types. In the same vein, it helps to store lists by converting java.sql.Array to a SQL Array.
Lastly, the PreparedStatement implements methods like getMetadata() that contain information about the returned result.

Mitigation Strategy 2] White-listing of the SQL Queries.

The SQL queries are scattered in different DAO classes, and it becomes difficult to maintain those SQL queries.
Firstly, we accumulate all the SQL queries into one class into a master SQL-specific class.
Secondly, we define a whitelist collection by including all of these SQL queries in there. This collection can be used in DAO implementations to verify if the SQL query they are executing is one of the whitelisted SQL queries or not.
[Also See https://community.veracode.com/s/article/allowlist-helps-fix-cwe ]

For example:

Class DbMaster{
public static String SQL_SEARCH_EMP =“SELECT * FROM EmployeeTbl WHERE id=?”;
public static Static String SQL_UPDATE_EMP = “UPDATE EmployeeTbl SET name=’?’ , email=’?’ WHERE id=?”;
public static String SQL_DELETE_EMP = “DELETE FROM EmployeeTbl WHERE id=?”;
public static String SQL_CHG_DEPT_OF_EMP = “UPDATE EmployeeTbl SET deptid=? WHERE id=?”;
public static String SQL_SEARCH_DEPT = “SELECT * FROM DepartmentTbl WHERE id=?”;
public static String SQL_CHG_HOD_OF_DEPT = “UPDATE DepartmentTbl SET hod=? WHERE id=?”;
public static String SQL_ALL_EMP_OF_LOCATION = “SELECT d.DepartmentName, e.name , e.designation, d.hod From DepartmentTbl d, EmployeeTbl e WHERE d.locationId=? AND d.id = e.deptID”;

private static List allSqlWhiteList= new LinkedList<String>();
static{
allSqlWhiteList.add(SQL_SEARCH_EMP);
allSqlWhiteList.add(SQL_UPDATE_EMP);
allSqlWhiteList.add(SQL_DELETE_EMP);
allSqlWhiteList.add(SQL_CHG_DEPT_OF_EMP);
allSqlWhiteList.add(SQL_SEARCH_DEPT);
allSqlWhiteList.add(SQL_CHG_HOD_OF_DEPT);
allSqlWhiteList.add(SQL_ALL_EMP_OF_LOCATION);
. . .
}
public List<String> getWhiteListedSQLs(){
return Collections.unmodifiableList(allSqlWhiteList);
}
}

In addition, have the Dao Implementation refer to this class to reference the SQL query. We also doing whitelist validation by adding a condition to check against the whitelist collection of SQL queries.

Example:

Class EmployeeDaoImpl{
Public Employee searchEmployee(int id){
.. .. .. 
String query = DbMaster.SQL_SEARCH_EMP;                 //Refer the SQL from DBMaster instead of defining new SQL String.
if(DbMaster.getWhiteListedSQLs().contains(query)){      // Also, Validate the referred SQL is Whitelisted SQL
    PreparedStatement pstmt = connection.prepareStatement(query);
    ResultSet  resultSet = null;    
    pstmt.setString(1,employeeId);
    resultSet = pstmt.executeQuery();
}
.. .. .. 
}
Public int updateEmployee(Employee emp){
.. .. .. 
String query = DbMaster.SQL_UPDATE_EMP;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstatement = connection.prepareStatement(query); 
    Pstatement.setString(1,employee.getName());
    Pstatement.setString(2,employee.getEmail());
    Pstatement.setString(3,employee.getId());
    pstatement.executeUpdate(); 
}
.. .. .. 
}
Public int deleteEmployee(Employee emp){
.. .. .. 
String query = DbMaster.SQL_DELETE_EMP;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstatement = connection.prepareStatement(query); 
    Pstatement.setString(1,employee.getId());
    pstatement.executeUpdate();
}
.. .. .. 
}
Public int changeDepartment(Employee emp, int newDeptId){
.. .. .. 
String query = DbMaster.SQL_CHG_DEPT_OF_EMP;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstatement = connection.prepareStatement(query);
    Pstatement.setString(1,newDeptId);
    Pstatement.setString(2,employee.getId());
    pstatement.executeUpdate();
}
.. .. .. 
}
}

I can have all the other Dao Implementations refer to the same Master SQL class for whitelisted SQL collection.

Class DepartmentDaoImpl{
Public Department getDepartmentDetails(int Deptid){
.. .. .. 
String query = DbMaster.SQL_SEARCH_DEPT;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstmt = connection.prepareStatement(query);
    ResultSet  resultSet = null;    
    pstmt.setString(1,Deptid);
    resultSet = pstmt.executeQuery();
}
.. .. .. 
}
Public int changeHOD(int deptId, Employee hod_employee){
.. .. .. 
String query = DbMaster.SQL_CHG_HOD_OF_DEPT;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstatement = connection.prepareStatement(query);
    Pstatement.setString(1,deptId);
    Pstatement.setString(2,hod_employee.getId());
    pstatement.executeUpdate();
}

.. .. .. 
}
Public Object[] getAllEmployeesForLocation(int locationid){
.. .. ..
String query = DbMaster.SQL_ALL_EMP_OF_LOCATION;
if(DbMaster.getWhiteListedSQLs().contains(query)){
    PreparedStatement pstmt = connection.prepareStatement(query);
    ResultSet  resultSet = null;    
    pstmt.setString(1,locationid);
    resultSet = pstmt.executeQuery();

.. .. ..
}
}

Mitigation Strategy 3] Centralized Database Utility Design Pattern

Many application developers use a design pattern where they just pass SQL query string as an argument to a database-specific class for execution.
This class is meant to take care of the aspect of DB connection and execution of the queries blindly.
For example:

Class SQLHelper{
public static Object[] executeSqlQuery(String query ){
Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery(query);
.. .. .. 
return results;
}
public static int executeSqlUpdate(String query ){
Statement stmt = con.createStatement();
Int count = stmt.executeUpdate(query);
return count;
}
}

And these functions are then used in business logic (Dao layer classes) For e.g.

Class EmployeeDaoImpl{
Public Employee searchEmployee(int id){
.. .. .. 
String query = “SELECT * FROM EmployeeTbl WHERE id=”+id;
Employee result = SQLHelper.executeSqlQuery(query);
.. .. .. 
}
Public int updateEmployee(Employee emp){
.. .. .. 
String query = “UPDATE EmployeeTbl SET name=’”+emp.getName()+”’ , email=’”+emp.getEmail()+”’ WHERE id=”+emp.getId()+””;
int cnt = SQLHelper.executeSqlUpdate(query);
.. .. .. 
}
Public int deleteEmployee(Employee emp){
.. .. .. 
String query = “DELETE FROM EmployeeTbl WHERE id=”+emp.getId()+””;
int cnt = SQLHelper.executeSqlUpdate(query);
.. .. .. 
}
Public int changeDepartment(Employee emp, int newDeptId){
.. .. .. 
String query = “UPDATE EmployeeTbl SET deptid=”+newDeptId +” WHERE id=”+emp.getId()+””;
int cnt = SQLHelper.executeSqlUpdate(query);
.. .. .. 
}
 
}
 
Class DepartmentDaoImpl{
Public Department getDepartmentDetails(int Deptid){
.. .. .. 
String query = “SELECT * FROM DepartmentTbl WHERE id=”+Deptid;
Department result[] = SQLHelper.executeSqlQuery(query);
.. .. .. 
}
Public int changeHOD(int deptId, Employee emp){
.. .. .. 
String query = “UPDATE DepartmentTbl SET hod=’”+emp.getId()+”’ WHERE id=”+deptId+””;
int cnt = SQLHelper.executeSqlUpdate(query);
.. .. .. 
}
Public Employee[] getAllEmployeesForLocation(int locationid){
.. .. ..
String query = “SELECT d.DepartmentName, e.name , e.designation From DepartmentTbl d, EmployeeTbl e WHERE d.locationId =”+ locationid +” AND d.id = e.deptID “;
Employee result[] = SQLHelper.executeSqlQuery(query);
.. .. ..
}
}

With this design, The SQL Injection CWE 89 flaw will be flagged only on the SQLHelper.executeSqlQuery() and SQLHelper.executeSqlUpdate() and not on the Dao Implementation classes hence it becomes difficult to backtrack the root cause to identify which SQL query is problematic. We would need to manually review all the references of that function and assess the SQL queries passed as a String to that function.
Firstly, we need to rewrite the code to use Parameterized PreparedStatement. We would modify the program as :

Class SQLHelper {
public static Object[] executeSqlQuery(String query, List params){
PreparedStatement pstmt = con.prepareStatement(query);
For(int i=0;i<params.size();i++){
    pstmt.setString((i+1), params.get(i));  //We are only using setString() for demo purpose, it can of different data type.
}
ResultSet rs = pstmt.executeQuery();
.. .. .. 
return results;
}
public static int executeSqlUpdate(String query, List params){
PreparedStatement pstmt = con.prepareStatement(query);
For(int i=0;i<params.size();i++){
    pstmt.setString((i+1), params.get(i));
}
 
Int count = pstmt.executeUpdate();
return count;
}
}

And uses these functions in business logic (Dao layer classes) For e.g

Class EmployeeDaoImpl{
Public Employee searchEmployee(int id){
.. .. .. 
String query = “SELECT * FROM EmployeeTbl WHERE id=?”;
List params = new LinkedList();
Params.add(id);
Employee result = SQLHelper.executeSqlQuery(query,params);
.. .. .. 
}
Public int UpdateEmployee(Employee emp){
.. .. .. 
String query = “UPDATE EmployeeTbl SET name=’?’ , email=’?’ WHERE id=?”;
List params = new LinkedList();
Params.add(emp.getName());
Params.add(emp.getEmail());
Params.add(emp.getId());
 
int cnt = SQLHelper.executeSqlUpdate(query,params);
.. .. .. 
}
 
Public int deleteEmployee(Employee emp){
.. .. .. 
String query = “DELETE FROM EmployeeTbl WHERE id=?”;
List params = new LinkedList();
Params.add(emp.getId());
int cnt = SQLHelper.executeSqlUpdate(query,params);
.. .. .. 
}
}

But by doing that also, it does not guarantee that query that you are passing as a String argument to the functions SQLHelper.executeSqlQuery() and SQLHelper.executeSqlUpdate() is not dynamically constructed from untrusted variables.

It is possible the string is partially constructed using variable and uses ‘?’

For e.g.

String query = “UPDATE EmployeeTbl SET name=’?’, email=’?’,type=”+ type +” WHERE id=?”;

In the above example, the query is partially parameterized but still uses a variable ‘type’ which can be tainted, coming from untrusted sources, and can result in SQL Injection.
So, It is best to combine the centralized DB Execution utility with the Whitelisted set of SQL mitigation strategies.
[Also See: https://community.veracode.com/s/article/allowlist-helps-fix-cwe ]

Mitigation Strategy 4] Combined centralized DB utility & Whitelisted set of SQL.

In the below example, the Dao implementation classes are separating both the aspects of writing SQL queries as well as the database execution from itself.
Here, we just need a reference of the SQL query from DbMaster and maintain a list of Parameters for that SQL query.
The class SQLHelper does the job of ensuring that the SQL query being executed is one of the whitelisted SQL and correctly parametrizing the PreparedStatement before executing it.

Class SQLHelper {

public static Object[] executeSqlQuery(String query, List params){
.. .. .. 
if(DbMaster.getWhiteListedSQLs().contains(query)){
	PreparedStatement pstmt = con.prepareStatement(query);
	For(int i=0;i<params.size();i++){
		pstmt.setString((i+1), params.get(i));
	}
	ResultSet rs = pstmt.executeQuery();
}
.. .. .. 
}

public static int executeSqlUpdate(String query ){
.. .. ..
if(DbMaster.getWhiteListedSQLs().contains(query)){
	PreparedStatement pstmt = con.prepareStatement(query);
	For(int i=0;i<params.size();i++){
		pstmt.setString((i+1), params.get(i));
	}
 	int count = pstmt.executeUpdate();
}
.. .. 
}
}

Dao Implementation looks like:

Class EmployeeDaoImpl{
Public Employee searchEmployee(int id){
.. .. .. 
String query = DbMaster.SQL_SEARCH_EMP;
List params = new LinkedList();
Params.add(id);
Employee result = SQLHelper.executeSqlQuery(query,params);
.. .. .. 
}
Public int UpdateEmployee(Employee emp){
.. .. .. 
String query = DbMaster.SQL_UPDATE_EMP;
List params = new LinkedList();
Params.add(emp.getName());
Params.add(emp.getEmail());
Params.add(emp.getId());
 
int cnt = SQLHelper.executeSqlUpdate(query,params);
.. .. .. 
} 
Public int deleteEmployee(Employee emp){
.. .. .. 
String query = DbMaster.SQL_DELETE_EMP;
List params = new LinkedList();
Params.add(emp.getId());
int cnt = SQLHelper.executeSqlUpdate(query,params);
.. .. .. 
}
}

Class DepartmentDaoImpl{
Public Department getDepartmentDetails(int Deptid){
.. .. .. 
String query = DbMaster.SQL_SEARCH_DEPT;
List params = new LinkedList();
Params.add(Deptid);
Department result[] = SQLHelper.executeSqlQuery(query,params);
.. .. .. 
}
Public int changeHOD(int deptId, Employee emp){
.. .. .. 
String query = DbMaster.SQL_CHG_HOD_OF_DEPT;
List params = new LinkedList();
Params.add(emp.getId());
Params.add(Deptid);
int cnt = SQLHelper.executeSqlUpdate(query,params);
.. .. .. 
}
.. .. ..
}

This way we guarantee that the ‘query’ passed as a String argument to the executing utility function is always safe and can be trusted.

Click the below link for other mitigation strategies:
https://community.veracode.com/s/article/sql-injection-mitigation-page2

Mitigation Strategy 5] Dynamic Table names and Columns names

How to fix CWE 89 SQL Injection flaws?

Mitigation Strategy 1] Using Prepared Statement.

Mitigation Strategy 2] White-listing of the SQL Queries.

Mitigation Strategy 3] Centralized Database Utility Design Pattern

Mitigation Strategy 4] Combined centralized DB utility & Whitelisted set of SQL.

Mitigation Strategy 6] Setting up for PreparedStatement using IN Clause

Mitigation Strategy 7] ESAPI Escaping using Database-specific codec.

Mitigation Strategy 8] Encoding the untrusted data.

Mitigation Strategy 9] Using Stored Procedure

Mitigation Strategy 10] Veracode Custom cleanser annotation

Mitigation Strategy 11] Defense In-depth.

Topics (10)

Related Topics

Getting Started

Developer Tools

Resources

Help & Support

Community

Product

Veracode Verified

News

Strictly Necessary Cookies

Performance Cookies

Targeting Cookies

How to fix CWE 89 SQL Injection flaws?

Mitigation Strategy 1] Using Prepared Statement.

Mitigation Strategy 2] White-listing of the SQL Queries.

Mitigation Strategy 3] Centralized Database Utility Design Pattern

Mitigation Strategy 4] Combined centralized DB utility & Whitelisted set of SQL.

Mitigation Strategy 6] Setting up for PreparedStatement using IN Clause

Mitigation Strategy 7] ESAPI Escaping using Database-specific codec.

Mitigation Strategy 8] Encoding the untrusted data.

Mitigation Strategy 9] Using Stored Procedure

Mitigation Strategy 10] Veracode Custom cleanser annotation

Mitigation Strategy 11] Defense In-depth.

Topics (10)

Related Articles

Related Topics

Getting Started

Developer Tools

Resources

Help & Support

Community

Product

Veracode Verified

News