MESTIOCO241725 (Community Member) asked a question.
StringBuffer insertBufferOriginal = new StringBuffer("UPDATE " + entity.name() + " SET ");
String update = "UPDATE #TABLE SET ";
String table = entity.name();
update = update.replace("#TABLE", table);
StringBuffer insertBuffer = new StringBuffer(update);
boolean appendComma = false;
for(Tuple<String, String> fieldName : fieldNames) {
if(!fieldName.first.equals("id")) {
if(appendComma) {
insertBuffer.append(",");
} else {
appendComma = true;
}
insertBuffer.append(fieldName.getSecond()).append(" = ?");
}
}
insertBuffer.append(whereClause);
//prepared stmt
jdbcTemplate.update(insertBuffer.toString(), classObjects); //SQL injection reported here by Veracode. not sure why dynamic queries causing SQL Injection issue.
.png){kind=logo}
Hi @MESTIOCO241725 (Community Member),
Without more context, it is hard to know what exactly causes the SQL Injection finding here. However, one thing that stands out already is the `replace` on line 4. If this variable `table` (or `entity.name()`) can come from outside the application (such as an HTTP request, the filesystem, or even the database) you may still have SQL Injection despite using prepared statements. In this case, we would recommend using a strict allow-list to validate all dynamic parts of the query that cannot be parameterized (e.g. table names, schema names, column names, etc.).
It might also be worth reviewing the flaw description which may provide more information on where the data triggering this finding originates.
Thank you,
Florian Walter (Veracode)
String update = "UPDATE #TABLE SET ";
String table = "RELEASE_INFO";
update = update.replace("#TABLE", table);
StringBuffer insertBuffer = new StringBuffer(update);
boolean appendComma = false;
for(Tuple<String, String> fieldName : fieldNames) {
if(!fieldName.first.equals("id")) {
if(appendComma) {
insertBuffer.append(",");
} else {
appendComma = true;
}
insertBuffer.append(fieldName.getSecond()).append(" = ?");
}
}
insertBuffer.append(whereClause);
//prepared stmt
jdbcTemplate.update(insertBuffer.toString(), classObjects);
Veracode Flaw message as below:
Type: org.springframework.jdbc.core.JdbcTemplate.update
Category: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
The flaw description usually contains more information about the origin of the potentially untrusted data (e.g. "The tainted data originated from [...]") which can help to triage the flaw. Can you see this additional information in the "Triage Flaws" view in the Veracode platform?
I can see now that a hardcoded table name still leads to the finding. Another thing worth investigating in the code snippet is the variable `whereClause`, which is appended to the query. Could you provide more information on how this is being used?
Thank you,
Florian Walter
Thank you Florian,
StringBuffer whereClause = new StringBuffer(" where "); --> For whereClause. doing simple replacing key used in my program still showing as an issue.
Veracode details from Triage Flaws:
High Flaw
Flaw Id: 72
Description: This database query contains a SQL injection flaw. The call to org.springframework.jdbc.core.JdbcTemplate.update() constructs a dynamic SQL query using a variable derived from untrusted input. An attacker could exploit this flaw to execute arbitrary SQL queries against the database. The first argument to update() contains tainted data. The tainted data originated from an earlier call to com.finmechanics.uobire.UploadMain.main.
Remediation: Avoid dynamically constructing SQL queries. Instead, use parameterized prepared statements to prevent the database from interpreting the contents of bind variables as part of the query. Always validate untrusted input to ensure that it conforms to the expected format, using centralized data validation routines when possible.
Hi @MESTIOCO241725 (Community Member) ,
I can't see any parameters of the prepared statement being set in the code snippet. Also, if the variable `fieldNames` would contain data that can come from outside the application, there would still be risks despite the use of prepared statements.
Thank you,
Florian Walter
Hi Florian,
If you look into my code which I provided where I'm preparing prepared statement query and store it into 'insertBuffer.append(fieldName.getSecond()).append(" = ?");' and then finally calling jdbcTemplate.update(insertBuffer.toString(), classObjects);. Want to understand why dynamically created prepared statement causing sql injection issues.
Full source code:
public void update(T pojo, Class clazz, JdbcTemplate jdbcTemplate) throws Exception{
List<Tuple<String, String>> fieldNames = getFieldNames(clazz);
StringBuffer whereClause = new StringBuffer(" where ");
Object[] classObjects = new Object[fieldNames.size()];
int icount = 0;
for(Tuple<String, String> tuple :fieldNames) {
try {
Object obj = FieldUtils.readDeclaredField(pojo, tuple.first, true);
if(tuple.first.equals("id")) {
whereClause.append(tuple.second).append(" = ?");
classObjects[fieldNames.size()-1] = obj;
} else {
if (obj!=null && obj.getClass().equals(Boolean.class)){
obj = Boolean.TRUE.equals(obj) ? "Y" : Boolean.FALSE.equals(obj) ? "N" : null;
}
if (obj!=null && obj.getClass().isEnum()){
obj = obj.toString();
}
classObjects[icount++] = obj;
}
} catch (IllegalAccessException e) {
logger.error(e);
}
}
Entity entity = (Entity) clazz.getAnnotation(Entity.class);
//SQL vulnerability fix
StringBuffer insertBufferOriginal = new StringBuffer("UPDATE " + entity.name() + " SET ");
String update = "UPDATE #TABLE SET ";
String table = entity.name();
update = update.replace("#TABLE", table);
StringBuffer insertBuffer = new StringBuffer(update);
boolean appendComma = false;
for(Tuple<String, String> fieldName : fieldNames) {
if(!fieldName.first.equals("id")) {
if(appendComma) {
insertBuffer.append(",");
} else {
appendComma = true;
}
insertBuffer.append(fieldName.getSecond()).append(" = ?"); //prepare prepared statement query
}
}
insertBuffer.append(whereClause);
//prepared stmt
jdbcTemplate.update(insertBuffer.toString(), classObjects); //Trying to insert into DB but Veracode reporting this as a High flaw for SQL injection as this is not.
}
Are you suggesting to prepare query like --> UPDATE RELEASE_INFO SET COL1= ? WHRE CONDITION?
but what I designed is to prepare for all columns using a for loop instead of manually passing columns into 'UPDATE SET' by separating columns using ',' so that it will support in case of any new columns added.
Final Query in String Buffer Object: UPDATE RELEASE_INFO SET COL1= ?, COL2=?, COL3=? WHRE CONDITION1=10 AND CONDITION2=2021; --> This query I'm preparing above and store it into StringBuffer. I'm not seeing anything wrong with code causes SQL Injection problem here.
Hi @MESTIOCO241725 (Community Member) ,
An example where prepared statements alone wouldn't be sufficient would be if the function `getFieldNames()` would return data from outside the application, such as an HTTP request, your database, or the file system. Binding external data is never sufficient if any non-parameterizable part of the query (such as the column names) is dynamically created with external input.
I would recommend you schedule a consultation call to further discuss this. You can check out this knowledge article (https://community.veracode.com/s/article/How-to-schedule-a-consultation-call) on how to schedule a consultation call with us.
Thank you,
Florian Walter