javaspringspring-bootsql-injectioncheckmarx

Can I use if statements to prevent SQL injection?


I have been trying to get my application to pass Checkmarx but I keep running into a Second Order SQL Injection vulnerability.

Presently my code looks like this:

    public static final String SELECT_NEXTVAL_FOR = "SELECT NEXTVAL FOR ";

    @Value("#{ systemProperties['sequence.schema']}")
    String sequenceSchema;

    public Integer getNextContactId() {
        if (sequenceSchema.matches("[A-Z]{3}@[A-Z]{3}|[A-Z]\\d[A-Z]{5}")) {
            Query getNextContactIdQuery = entityManager.createNativeQuery(
                    SELECT_NEXTVAL_FOR + sequenceSchema + ".CNTCT_ID FROM SYSIBM.SYSDUMMY1");

            return (Integer) getNextContactIdQuery.getSingleResult();
        } else {
            return null;
        }

Checkmarx is flagging the sequenceSchema as the problem here. My thoughts were that the if statement would ensure whatever is present in sequenceSchema would only match the specific regex for our schemas. Apparently that is not good enough. I have tried to use positional/named parameters but those don't work outside of WHERE or HAVING clauses. I have tried Criteria Queries as seen here without any luck.

I had another thought along similar lines to what I have here, but instead of using regex I could create a set of the valid schemas and comparing that to the sequenceSchema as such:

    public static final String SELECT_NEXTVAL_FOR = "SELECT NEXTVAL FOR ";

    @Value("#{ systemProperties['sequence.schema']}")
    String sequenceSchema;

    Set<String> validSchemas = ... //create set of valid schemas

    public Integer getNextContactId() {
        if (validSchemas.contains(sequenceSchema)) {
            Query getNextContactIdQuery = entityManager.createNativeQuery(
                    SELECT_NEXTVAL_FOR + sequenceSchema + ".CNTCT_ID FROM SYSIBM.SYSDUMMY1");

            return (Integer) getNextContactIdQuery.getSingleResult();
        } else {
            return null;
        }

Would that prevent against SQL injection? If not, why? I am really struggling to find a working solution here.


Solution

  • Folks who recommend using query parameters to defend against SQL injection are half-right. That is the recommended solution in most cases of dynamic SQL queries.

    But query parameters can be used only in place of constant values. Not identifiers (in your case a schema identifier), or expressions, or SQL keywords, etc.

    You are on the right track with using validation of some kind, so the schema identifier is limited to a known safe value. Then you can interpolate that string into your SQL query. If you have done adequate validation, then it's safe from SQL injection.

    You should also delimit your identifier, just in case it contains a space, or a punctuation symbol, or conflicts with an SQL reserved keyword. In DB2, use double-quotes as identifier delimiters.

    However, even though the kind of validation you are doing may be sufficient, it may not be a method of defense that can be recognized by an automated SQL injection detection tool like Checkmarx. It might see that you're putting variables together into an SQL string, and conclude that it's an SQL injection risk. It can't tell that your validation is sufficient to prevent problems.

    Automated solutions for SQL injection detection are always limited. You do have to use human analysis sometimes.

    I know this is unfashionable to say these days, given the trend to say that AI will put programmers out of work. But AI is not yet able to handle edge cases as reliably as expert humans. I am not convinced it ever will be.