0

I am running into an issue where Checkmarx is flagging one of my Services as having a Second Order SQL Injection vulnerability. I have attempted to do some research to figure out how to resolve this but most solutions seem to be around queries that are using parameters. This query that is being flagged does not use any parameters. The code in question is here:

public static final String SELECT_NEXTVAL_FOR = "SELECT NEXTVAL FOR ";
public static final String FROM_SYSIBM = " FROM SYSIBM.SYSDUMMY1";
public static final String CONTACT_ID_TABLE = ".CNTCT_ID";
@Autowired
EntityManager entityManager;

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

public Integer getNextContactId() {
    Query getNextContactIdQuery = entityManager.createNativeQuery(
            SELECT_NEXTVAL_FOR + sequenceSchema + CONTACT_ID_TABLE + FROM_SYSIBM);

    return (Integer)getNextContactIdQuery.getSingleResult();
}

The message that Checkmarx is giving is here:

The application's getNextContactId method executes an SQL query with BinaryExpr, at line 27 of src\main\java\com\ford\fcnadealerdata\cloc\service\SequenceService.java. The application constructs this SQL query by embedding an untrusted string into the query without proper sanitization. The concatenated string is submitted to the database, where it is parsed and executed accordingly.

The attacker may be able to write arbitrary data to the database, which is then retrieved by the application with get in sequenceSchema; method at line 23 of service\SequenceService.java. This data then flows through the code, until it is used directly in the SQL query without sanitization, and then submitted to the database server for execution.

This may enable a Second-Order SQL Injection attack

It seems to have an issue with the + in between CONTACT_ID_TABLE and FROM_SYSIBM. As I mentioned all of the similar solutions I found involved parameters being passed in to the queries, here we aren't doing that so I don't think that I need something quite as complicated, though I could be wrong.

scottyboombox
  • 25
  • 5
  • Sounds to me like a false positive Checkmarx finding, similar to [this](https://stackoverflow.com/questions/72001344/sql-injection-prevention-in-spring-boot) one. – Woodchuck Apr 26 '23 at 14:52
  • True Positive. The string concatenation with a value obtained from systemProperties to form the native query is the injection attack vector. – NathanL Apr 26 '23 at 15:47
  • @NathanL So from what I've read a solution to this seems to be possible using a parameterized query instead of the concatenation. Such as this: `entityManager.createNativeQuery("SELECT NEXTVAL FOR ? FROM SYSIBM.SYSDUMMY1.CNTCT_ID").setParameter(1, sequenceSchema)` – scottyboombox Apr 26 '23 at 20:14
  • @scottyboombox it might work, assuming you can bind a value at that location in that statement. – NathanL Apr 27 '23 at 00:51
  • @NathanL Well you were right about not being able to bind a value at that location. Back to square one... – scottyboombox Apr 27 '23 at 17:38

0 Answers0