Safe Coding Practices

SQL Injection

There are also several different types of SQL injection that can cause significant problems for database-centric applications. In some cases, attackers are simply attempting to gain access to more information than they should normally see. In other cases, attackers are more concerned with storing new information in the database that will then be used naively by the application at a later date to compromise the end user's session.

Query-based attacks focus on a prevalent antipattern that involves constructing queries on the fly using string concatenation. This vulnerability type shows up most frequently in web-facing applications, and is equally visible in all the usual page stacks—PHP, ASP, JSP, and so on—along with their backing controller logic.

The core of this vulnerability revolves around developers using direct query execution rather than query preparation to run database interaction. Consider this example of a login validation query:

SELECT ID FROM USERS WHERE NAME= 'user' AND PWD='password'

Users are presented with a simple HTML form containing two input boxes and using this antipattern. The incoming parameters from this form (however they're received by the page stack in question) are simply substituted into a string form of the query by concatenation.

Consider a set of parameters provided by an attacker:

NAME:   x
PWD:    x' OR '1' = '1

Run the concatenation and the result is the—now exploited—query:

SELECT ID FROM USERS WHERE NAME=
  'x' AND PWD='x' OR '1' = '1'

If this is compounded by the login simply checking for success or failure of this statement's execution (as opposed to counting result rows), attackers are quickly granted whatever access rights might be available from whatever user records are processed by the application. In applications where the first row of the user table is reserved for the superuser, the application could easily be completely compromised.

There are many other forms of attack possible using applications that are not careful in their treatment of substitution strings within database statements. As common as this antipattern is (see recent announcements from Microsoft and others to see the prevalence that's out there), the mitigation is very simple and is built into basic database APIs: Use prepared statements, not string concatenation.

For example, consider the incorrect implementation in Example 2. This function follows the antipattern rigorously, and also performs another significant no-no by throwing an exception that includes incoming (unfiltered) data—the user name. Put this data up in front of the user as a response and you're open to several knock-on exploits, notably the potential for cross-site scripting.

public void validateUser(String user, String pwd, Connection db)
    throws InvalidUserException
{
  Statement stmt = null;
  ResultSet rs = null;
  try
  {
      // Create the statement
      stmt = db.createStatement();
      String sql = "select id from users where user='" + user +
                   "' and pwd='" + pwd + "'";
      // Execute it, process the result
      rs = stmt.executeQuery(sql);
      if( rs == null || rs.next() == null )
          throw new InvalidUserException(user);
  }
  catch( SQLException e )
  {
      throw new InvalidUserException(user);
  }
  finally
  {
    try { if( rs != null ) rs.close(); } catch( Exception e ) { }
    try { if( stmt != null ) stmt.close(); } catch( Exception e ) { }
  }
}

To fix this code, instead of constructing the SQL query on the fly, simply construct a prepared statement and then use it to substitute the incoming parameters.

The statement that we're going to prepare reserves space for parameters and is not vulnerable to this exploit because it isn't lexically brittle in the same way as string concatenation.

Consider this statement (which I'll prepare for the same purpose as the aforementioned concatenated string):

SELECT ID FROM USERS WHERE USER=?
   AND PWD=?

I use this prepared statement to substitute our incoming data into the user and pwd parameter reservations. If we provide the same previously exploited strings as input, the result will be a failure during query substitution, as you can't provide an argument to a prepared query that includes metacharacters like the single quote.

Other potential exploits will be caught at different stages, but as you can see the new implementation, as in Example 3, is just as simple to create as the original, but is now much safer (we've also removed the username from the thrown exception, to avoid any temptation to expose it unfiltered to the caller).

public void validateUser(String user, String pwd, Connection db)
    throws InvalidUserException
{
  PreparedStatement stmt = null;
  ResultSet rs = null;

  try
  {
      // Prepare the statement, rather than concatenating it
      String sql = "select id from users where user=? and pwd=?");
      stmt = db.prepareStatement(sql);

      // Substitute our incoming parameters into the query
      stmt.setString(1, user);
      stmt.setString(2, pwd);

      // Execute the query and process the results as before
      rs = stmt.executeQuery();
      if( rs == null || rs.next() == null )
          throw new InvalidUserException();
  }
  catch( SQLException e )
  {
      throw new InvalidUserException();
  }
  finally
  {
    try { if( rs != null ) rs.close(); } catch( Exception e ) { }
    try { if( stmt != null ) stmt.close(); } catch( Exception e ) { }
  }
}

In general, whether working with queries or DML, when dealing with data coming from the end user, always use prepared statements to take advantage of filtering and parsing built into the database itself.