stoty commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1164009421
##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2563,13 +2618,17 @@ private static void
updateIndexesSequenceIfPresent(PhoenixConnection connection,
String newSchemaName =
MetaDataUtil.getViewIndexSequenceSchemaName(physicalName, true);
String newSequenceName =
MetaDataUtil.getViewIndexSequenceName(physicalName, tenantId, true);
// create new entry with new schema format
- String upsert = "UPSERT INTO " +
PhoenixDatabaseMetaData.SYSTEM_SEQUENCE + " SELECT NULL,\'" + newSchemaName +
- "\',\'" + newSequenceName
- + "\'," + START_WITH + "," + CURRENT_VALUE + "," +
INCREMENT_BY + "," + CACHE_SIZE + "," + MIN_VALUE
- + "," + MAX_VALUE + "," + CYCLE_FLAG + "," +
LIMIT_REACHED_FLAG + " FROM "
- + PhoenixDatabaseMetaData.SYSTEM_SEQUENCE + " WHERE " +
PhoenixDatabaseMetaData.TENANT_ID
- + " IS NULL AND " + PhoenixDatabaseMetaData.SEQUENCE_SCHEMA +
" = '" + oldSchemaName + "'";
- connection.createStatement().executeUpdate(upsert);
+ String upsert = "UPSERT INTO " +
PhoenixDatabaseMetaData.SYSTEM_SEQUENCE
+ + " SELECT NULL,\'" + newSchemaName + "\',\'" + newSequenceName
Review Comment:
newSchemaName and newSequencename.
Any concatenated non-constant string string is potentially vulnerable to sql
injection.
These seem to be the schema and name of sequences.
While realistically it's unlikely to be a real security hole,
We cannot guarantee without auditing a lot of code that the existing
sequence names don't resolve to a string that can be used for second order sql
injection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
