HQL SQL Injection Prevention

Contents 1 Introduction 2 Steps of an SQL/HQL injection attack 3 How applications are vulnerable to HQL/SQL injection? 4 Recommendations / Guidelines to prevent injections 4.1 HQL queries 4.2 SQL queries 4.2.1 SQLC 4.2.1.1 Optional parameters of type argument 4.2.1.2 Optional parameters of type replace 4.2.2 Stored procedures 5 Input filtering 5.1 Whitelist 5.2 Filters 6 Extra information 6.1 Prepare HQL queries for automated security checks 6.2 Error messages 7 External links with more information

Introduction

SQL/HQL injection is a code injection technique which provides an attacker with DataBase data that should never be revealed otherwise. It is based on SQL/HQL statements that are externally inserted through unsafe queries, without being filtered nor cleaned beforehand. These statements end up concatenated in the queries. As a result of this type of attack, identity spoofing, data alteration, complete disclosure of all data (personal or business related) or even gain full access to an administrator user can happen. In general, SQL/HQL Injection is considered to have a high impact severity.

Some of the main risks are:

• Confidentiality breach: access to sensitive information
• Authentication and authorization breach: possibility to use another user without knowledge of its password
• Integrity risk: modify or delete data inside the DB ruining registers, transactions...

Steps of an SQL/HQL injection attack

The attacks based on SQL/HQL injection can be separated into two main steps:

• Research how the targeted DataBase functions. This is done by inserting random values and edge cases in an input field to see the errors that are thrown as responses.

• Now that the attacker has enough information about the DataBase and its functioning, the extraction or modification of the data begins. Examples:
  • In a window that allows to get the transactions of a specific customer entering its ID, the following argument "1000' OR 1=1; --" could be used to retrieve the records of transactions of all customers without knowing the ID of any of them. The "--" forces the last "’;" to not be executed and act as a comment.

    "SELECT * FROM M_TRANSACTION WHERE CustomerID='" + customerId + "';";

    Taking into account that customerId="1000' OR 1=1; --":

    SELECT * FROM M_TRANSACTION WHERE CustomerID='1000' OR 1=1; --'; 

  • Once the attacker knows that the table’s name for the registered users is named ‘USERS’, he wants to delete the entire table. It could easily be done in the login window with a password input like "password'; DROP TABLE USERS; --" in a query as the following one:

    "SELECT email FROM USERS WHERE email ='" + email + "' AND password='" + password + "';";

    As the password entered is "password'; DROP TABLE USERS; --", the query would end up being executed like:

    SELECT email FROM USERS WHERE email ='email@gmail.com' AND password='password'; DROP TABLE USERS; --'; 

  • Alternatively, the attacker wants to have admin access to the entire web. The same query as before could grant him access, using "password'; INSERT INTO USERS ('email', 'password', 'login_id', ‘admin’) VALUES ('email@gmail.com', 'password', fakeId, TRUE); --" as password:

    SELECT email FROM USERS WHERE email ='email@gmail.com' AND password='password'; INSERT INTO USERS ('email', 'password', 'login_id', ‘admin’) VALUES ('email@gmail.com', 'password', fakeId, TRUE); --'; 

How applications are vulnerable to HQL/SQL injection?

Injections of this kind might occur in every query (dynamic or not) that uses external input without previous sanitization or validation. That's why it's very important to always consider any external input as something potentially dangerous to be especially treated before building an HQL/SQL query.

Recommendations / Guidelines to prevent injections

The key to prevent this kind of attack from happening is to avoid mixing data with SQL query code.

To avoid this situation, a developer must:

1. Always use bind parameters whenever possible (either in SQL or HQL).
2. If it is not possible to use parameters, for HQL queries we must use OBCriteria and Criteria, as they are flexible solutions that allow dynamic modifications to queries.
3. If OBCriteria or Criteria are not an option or if the query is a SQL Query then the developer must ensure that the input parameters are verified first with techniques such as filtering and whitelists

The main idea of the effort to elude this kind of attack is that direct concatenation in queries must be avoided. There is no such thing as a safe value/parameter.

HQL queries

The most typical scenario for a developer is inserting parameters in the WHERE clause of a query, where it is highly recommended to pass them as parameters to the OBCriteria (or Criteria) HQL query , avoiding String concatenation.

Sometimes we are required to build dynamic queries that affect parts that can't even be inserted as a parameter to a query without using OBCriteria/Criteria, like for example in the FROM clause. Because of that restriction, they are usually directly concatenated to the String of the query, which is a known risk.

Instead of that dangerous practice (following bad example), we can safely use the OBCriteria or Criteria infrastructure, as they can be dynamically modified. In the following good example, the same structure is valid for executing the query using any class that extends from BaseOBObject.

Bad example(avoid):

...
// tableName, propertyName and propertyValue are direct user inputs
//@formatter:off
final String hql = 
              "select a "
              "  from " + tableName + " as a "
              "  where a." + propertyName + " = " + propertyValue ;
//@formatter:on
final Query<Object> query = OBDal.getInstance()
    .getSession()
    .createQuery(hql, Object.class);
...

In the following example, clazz acts as substitute to tableName.

Good example(recommended):

...
public <T extends BaseOBObject> List<T> getFilteredObjects(Class<T> clazz, String propertyName, String propertyValue) {
     final OBCriteria<T> obc = OBDal.getInstance().createCriteria(clazz);
     obc.add(Restrictions.eq(propertyName, propertyValue));
     return obc.list();
 }
...

SQL queries

SQLC

Instead of using manual jdbc code, SQL queries should be defined in SQLC, which are later translated into java code, in which some injections vulnerabilities can be found depending on the type of parameter used. Because of it, filtering parameters (whitelist) must be done before inserting them into queries.

In SQLC there are two groups of parameters: mandatory and optional:

1. Mandatory ones are already safe as they are validated before inserting them into any query.
2. Optional parameters allow you to add and/or modify parts of the SQL statement at runtime, but some of them are already safe, such as parameters without type and parameters of type none.

The problematic optional parameters are the following:

Optional parameters of type argument

This kind of parameter is completely ignored if the parameters' value is null or an empty String "". Otherwise the executed statement does include the fixed part of the parameter followed by the parameters value, without using a placeholder ("?"), using unsafe concatenation instead. This is why they need to be checked and cleaned before inserting them into the query. The following is an example of this, using filters (whitelist also applicable) as input validation method:

...
<!-- xsql definition -->
<Parameter name="parSalesOrders" optional="true" type="argument" after="WHERE 1=1" text="AND C_Order_ID IN"/>
...

...
// This is how it's transformed to Java
strSql +=  AND C_Order_ID IN" + parSalesOrders);
... 

...
// This is the call to the query transformed into Java with previous filtering
String strModules = vars.getInStringParameter("inpNodes", IsIDFilter.instance);
UpdateReferenceDataData[] data = UpdateReferenceDataData.selectModules(cp,strModules,strOrganization);
...

Optional parameters of type replace

These parameters allow to completely replace a section of an SQL statement with a new one. They are a very powerful tool, but also dangerous. Because of it, they need to be filtered/validated beforehand.

The following example is a safe implementation using filtering:

...
<!-- xsql definition -->
<Parameter name="modules" optional="true" type="replace" after="AD_MODULE_ID IN " text="('1')"/>
...

...
// This is how it's transformed to Java
strSql = strSql + modules;...

...
// This is the call to the query transformed into Java
String strModules = vars.getInStringParameter("inpNodes", IsIDFilter.instance);
UpdateReferenceDataData[] data = UpdateReferenceDataData.selectModules(cp, strModules, strOrganization);
...

Stored procedures

In the case of Stored procedures called from Java, it is recommended to execute them using CallStoredProcedure.java or CallProcess.java. These classes allows to safely call these procedures, binding the parameters.

Example of a safe call to a stored procedure using CallStoredProcedure.java:

...
private DocumentType getDoubtfulDebtDocumentType(Client client, Organization organization) {
    final List<Object> parameters = new ArrayList<>();
    parameters.add(client.getId());
    parameters.add(organization.getId());
    parameters.add("DDB");
    String strDocTypeId = (String) CallStoredProcedure.getInstance()
        .call("AD_GET_DOCTYPE", parameters, null);
...

However, whenever a stored function is directly called without using CallStoredProcedure/CallProcess, input validation must be implemented beforehand.

Input filtering

The idea is to run some custom validations on any external input before using it in a query. There are two main ways to process these values, which are the following:

Whitelist

They consist in defining validations in code that check if the parameter is between some valid options (whitelist) and throw an Exception or continue with execution of the query accordingly.

...
String tableName;
switch(parameterTableName):
  case "AD_USER": tableName = "AD_USER";
                 break;
  case "C_ORDER": tableName = "C_ORDER";
                 break;
  default : throw new InputValidationException("Nice try ;)");
...

Filters

They are already implemented for internal use and can be found in org.openbravo.base.filter package and all of them extend the class RequestFilter. With them, any input value can be validated through CalloutInfo and vars (as shown previously in SQLC examples via vars.getInStringParameter). The available filters in Openbravo are:

• IsIDFilter: Filter to check if the input value follows valid ID structure for Openbravo.
• IsPositiveIntFilter: Filter to check if the input value is a positive integer number.
• NumberFilter: Filter to check if a value can be parsed into a BigDecimal.
• RegexFilter: Filter to check if the input value matches a given regular expression.
• ValueListFilter: Filter to check if the input is contained in a fixed list of allowed input (Whitelist). The comparison is made case in-sensitively.

The following example shows a manual implementation (also possible) of how to verify that the parameter "inpId" inserted from an user actually follows a valid Id structure, to avoid any injection.

...
if (!IsIDFilter.instance.accept(id)) {
      log.error("Input: {} not accepted by filter: IsIDFilter", id);
      throw new OBException("Input: " + id + " is not an accepted input");
}
...

Extra information

Prepare HQL queries for automated security checks

Every HQL/SQL query must be a simple String constant, not a StringBuilder or StringBuffer, as there is no need to use them and they were a bad recommendation in the past.

This allows an automated tool to verify them against injection, which is a must, as we want to have every query checked automatically by the ongoing project for 21Q1.

In the following example, you can see that the query is a constant String but also, as described in HQL_Coding_Conventions, has the right formatting and indentation to improve readability.

...
// @formatter:off
final String qryStr = 
              "select t.table.id, wa.editableField" +
              "  from ADTab t" +
              "    left join t.window w" +
              "    left join w.aDWindowAccessList wa" +
              " where wa.role.id= :roleId";
// @formatter:on
final Query<Object[]> qry = SessionHandler.getInstance()
          .createQuery(qryStr, Object[].class)
          .setParameter("roleId", getRoleId());
final List<Object[]> tabData = qry.list();
...

Error messages

We must always limit output information to user in case of errors: messages shown to the user must not contain any information about variables, names (fields, tables, functions…) or any kind of functional error without formatting for users.

Example: In a login window, if an attacker tries to log in as “Admin” and password ‘1234’. If the password is not correct, the error message should never suggest that the user is valid but not the password.

Proper way: ‘The user, password or combination of both is not correct’.

External links with more information

1. OWASP Cheat Sheet Series - SQL Injection Prevention Cheat Sheet [1]
2. Netsparker- SQL Injection Cheat Sheet [2]
3. Baeldung - SQL Injection and How to Prevent It? [3]
4. Wikipedia - SQL injection [4]
5. UnixWiz - SQL Injection Attacks by Example [5]
6. Medium - Exploiting a HQL injection [6]