[
https://issues.apache.org/jira/browse/PHOENIX-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17711340#comment-17711340
]
ASF GitHub Bot commented on PHOENIX-6560:
-----------------------------------------
kabhishek4 commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1164018807
##########
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:
Thanks for the explanation. However, after parameterising the sequence name,
spotbug bug tool stopped issuing error for this statement. Do you think we
still need to consider this as a security hole? Probably, there could be
several other concatenations in this code and tool is not reporting
warning/error. Moreover, this PR is for addressing
SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE which is a high severity security
error. Considering this please let me know if we want to cover this as part of
this PR.
> Rewrite dynamic SQL queries to use Preparedstatement
> ----------------------------------------------------
>
> Key: PHOENIX-6560
> URL: https://issues.apache.org/jira/browse/PHOENIX-6560
> Project: Phoenix
> Issue Type: Improvement
> Components: core
> Reporter: Istvan Toth
> Assignee: Abhishek Kothalikar
> Priority: Major
>
> Most of the Phoenix code base already uses PreparedStatements, and adds all
> potentially vulnerable data as parameters.
> However, there are some places where we concatenate potentially problematic
> strings into the query.
> While most of those are constants and such, we should preferably pass all
> data as parameters to be on the safe side.
> (We still have to use dynamic strings for the preparedstatement strings, for
> handling things as is null, empty in clauses and such)
> Spotbugs marks these with SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE, so
> they're easy to find.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
