CWE 89: SQL Injection

Flaw

CWE 89: SQL Injection flaws occur when you create a SQL statement by building a String that includes untrusted data, such as input from a web form, cookie, or URL query-string.

For example:

String accountBalanceQuery =
  "SELECT accountNumber, balance FROM accounts WHERE account_owner_id = "
  + request.getParameter("user_id");

try {
    Statement statement = connection.createStatement();
    ResultSet rs = statement.executeQuery(accountBalanceQuery);
    while (rs.next()) {
        page.addTableRow(rs.getInt("accountNumber"), rs.getFloat("balance"));
    }
} catch (SQLException e) { ... }

Under normal operation, the user with ID 984 might be logged in, and go to the URL https://bankingwebsite/show_balances?user_id=984

This means that accountBalanceQuery would then be:

SELECT accountNumber, balance FROM accounts WHERE account_owner_id = 984

This is passed to the database, and the accounts and balances for user 984 are returned, and rows are added to the page to display.

The attacker could change the parameter “user_id” in the URL to:

https://bankingwebsite/show_balances?user_id=984%20OR%201%3D1

Which would cause the user_id parameter to be interpreted as:

984 OR 1=1

And this results in accountBalanceQuery being:

SELECT accountNumber, balance FROM accounts WHERE account_owner_id = 984 OR 1=1

When this query is passed to the database, it will return all the account numbers and balances it has stored, and rows are added to the page to display. The attacker now knows every user’s account numbers and balances.

Fix

You can fix this flaw by using a Prepared Statement to create a Parameterized Query as below:

String accountBalanceQuery =
  "SELECT accountNumber, balance FROM accounts WHERE account_owner_id = ?";

try {
    PreparedStatement statement = connection.prepareStatement(accountBalanceQuery);
    statement.setInt(1, request.getParameter("user_id"));
    ResultSet rs = statement.executeQuery();
    while (rs.next()) {
        page.addTableRow(rs.getInt("accountNumber"), rs.getFloat("balance"));
    }
} catch (SQLException e) { ... }

String accountBalanceQuery =
-  "SELECT accountNumber, balance FROM accounts WHERE account_owner_id = "
-  + request.getParameter("user_id");
+  "SELECT accountNumber, balance FROM accounts WHERE account_owner_id = ?";
 
 try {
-    Statement statement = connection.createStatement();
-    ResultSet rs = statement.executeQuery(accountBalanceQuery);
+    PreparedStatement statement = connection.prepareStatement(accountBalanceQuery);
+    statement.setInt(1, request.getParameter("user_id"));
+    ResultSet rs = statement.executeQuery();
     while (rs.next()) {
         page.addTableRow(rs.getInt("accountNumber"), rs.getFloat("balance"));
     }

If an attacker attempts to supply a value that’s not a simple integer, then statement.setInt() will throw a SQLException rather than permitting the query to complete.

Note that the query string is composed using a placeholder (the "?" in this example) for the user-supplied data, and then the user-supplied data is bound to the PreparedStatement object. This allows the database system (the combination of JDBC, driver, and the database server itself) to ensure that the query can execute safely.

References

CWE ↪ OWASP ↪ WASC ↪