Checkmarx shows the code has risks for second order injection

Question

Checkmark scanned our codes and showed these code have risks for second order injection the code like this

@SuppressWarnings("unchecked")
public List<Map<String, Object>> findBySQL(String sql, List<ScalarType> types, List<Object> values, Info info) throws ApplicationException {
    try {
        SQLQuery query = currentSession().createSQLQuery(sql);
        if (types != null) {
            for (ScalarType scalar : types) {
                query.addScalar(scalar.getColumn(), scalar.getType());
            }
        }

        if (values != null) {
            for (int i = 0; i < values.size(); i++) {
                query.setParameter(i, values.get(i));
            }
        }
        query.setResultTransformer(Transformers.ALIAS_TO_ENTITY_MAP);
        return query.list();
    } catch (Exception e) {
        throw new ApplicationException(e, info);
    }
}

Our code use the preparedStatement to execute sql. But why these code still have the risks, and how can I fix it?

How can this function guarantee that `sql` and the columns in `types` are safe? If the caller creates those objects using unsafe content, then the resulting query will be unsafe. I'm afraid a function like this `findBySQL()` that accepts arbitrary input is an SQL injection vulnerability by design. You could whitelist the individual columns i your `types` list, but the whole query in `sql` must be untrusted. — Bill Karwin, Jan 29 '19 at 15:17
The problem starts with the fact that `types` may be null. In fact avoid this function altogether. — Joop Eggen, Feb 07 '19 at 15:55

score 2 · Answer 1 · answered Feb 07 '19 at 15:24

The Checkmarx throws the error because the values you are setting to the query parameters are not validated for its type.

For example, let us assume the query formed with your PreparedStatement is as below and the value you want to pass to the query parameter is 'Test'

Select * from XYZ where COL1 = ?

If your code is compromised and if the intruder passes 'Test' OR 1 = 1 in the query parameter, then the condition will always be true and it would return all the records from the Table.

So, before executing the query you should validate all your inputs.

Hope this helps

score 0 · Answer 2 · answered Feb 07 '19 at 15:04

0

For solving your issue in Checkmarx, you need to validate that sql String

answered Feb 07 '19 at 15:04

SPoint

582
2
10

Checkmarx shows the code has risks for second order injection

2 Answers2