Skip to content

Commit

Permalink
Merge pull request #8489 from mandy-chessell/oak2024
Browse files Browse the repository at this point in the history
Fix RegEx queries in PostgreSQL Repository
  • Loading branch information
mandy-chessell authored Nov 13, 2024
2 parents ddf6d9d + 93f1186 commit 227dba4
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 499 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,34 +110,77 @@ public void setSearchString(String searchString)
* Return the SQL search string that needs to appear in the SQL query.
*
* @return fragment of SQL
* @throws RepositoryErrorException problem with the regEx
*/
private String getSearchStringClause()
private String getSearchStringClause() throws RepositoryErrorException
{
if (searchString != null)
{
/*
if (searchString.startsWith("\\Q") && (searchString.endsWith("\\E")))
{
return " and (" + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " = '" + searchString.substring(2, searchString.length()-2) + "') ";
}
*/

return " and (" + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " like '" + searchString + "') ";
return " and (" + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " ~ '" + this.getSafeRegex(searchString) + "') ";
}

return " ";
}


/**
* Check that a search string, which might contain a regEx expression, is safe to execute.
* Dangerous RegEx can be used in a denial of service attack. This simple time test ensures that the regEx is safe.
*
* @param suppliedSearchString the string to escape to avoid being interpreted as a regular expression
* @return string that is a safe regular expression
* @throws RepositoryErrorException the RegEx takes to long to execute
*/
private String getSafeRegex(Object suppliedSearchString) throws RepositoryErrorException
{
final String methodName = "getSafeRegex";
final String tester = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ\\E\\Q1234567890";

if (suppliedSearchString != null)
{
String strippedSearchString = suppliedSearchString.toString().replaceAll("(\\Q|\\E)", "");

/*
* Do not care about the result - just the time it takes
*/
Date currentTime = new Date();
Date maxExecutionTime = new Date(currentTime.getTime() + 500);
tester.matches(strippedSearchString);
Date completionTime = new Date();

if (completionTime.after(maxExecutionTime))
{
throw new RepositoryErrorException(PostgresErrorCode.BAD_REGEX.getMessageDefinition(searchString),
this.getClass().getName(),
methodName);
}

return strippedSearchString;
}

return null;
}


/**
* Set up the properties that should be matched during the query.
*
* @param matchProperties Optional list of entity properties to match (where any String property's value should
* be defined as a Java regular expression, even if it should be an exact match).
* @param matchCriteria Enum defining how the match properties should be matched to the entities in the repository.
* @param propertyTableName name of table holding the properties
* @throws RepositoryErrorException error in one of the supplied RegEx
*/
public void setMatchProperties(InstanceProperties matchProperties,
MatchCriteria matchCriteria,
String propertyTableName)
String propertyTableName) throws RepositoryErrorException
{
if ((matchProperties != null) &&
(matchProperties.getPropertyCount() > 0 ||
Expand All @@ -153,7 +196,6 @@ public void setMatchProperties(InstanceProperties matchProperties,
OpenMetadataProperty.EFFECTIVE_FROM_TIME.name,
matchProperties.getEffectiveFromTime());
propertyConditions.add(propertyCondition);

}

if (matchProperties.getEffectiveToTime() != null)
Expand All @@ -174,46 +216,20 @@ public void setMatchProperties(InstanceProperties matchProperties,
PropertyCondition propertyCondition = new PropertyCondition();

propertyCondition.setProperty(propertyName);
propertyCondition.setValue(instancePropertyValue);

if (instancePropertyValue instanceof PrimitivePropertyValue primitivePropertyValue)
{
if (primitivePropertyValue.getPrimitiveDefCategory() == PrimitiveDefCategory.OM_PRIMITIVE_TYPE_STRING)
{
if (primitivePropertyValue.getPrimitiveValue() != null)
if (matchCriteria == MatchCriteria.NONE)
{
String stringValue = primitivePropertyValue.getPrimitiveValue().toString();

if (stringValue.startsWith("\\Q"))
{
/*
* PostgreSQL does not like the use of the literal control characters
*/
primitivePropertyValue.setPrimitiveValue(stringValue.substring(2, stringValue.length()-2));

if (matchCriteria == MatchCriteria.NONE)
{
propertyCondition.setOperator(PropertyComparisonOperator.NEQ);
}
else
{
propertyCondition.setOperator(PropertyComparisonOperator.EQ);
}
}
else
{
if (matchCriteria == MatchCriteria.NONE)
{
propertyCondition.setOperator(PropertyComparisonOperator.NOT_LIKE);
}
else
{
propertyCondition.setOperator(PropertyComparisonOperator.LIKE);
}

}
propertyCondition.setOperator(PropertyComparisonOperator.NOT_LIKE);
}
else
{
propertyCondition.setOperator(PropertyComparisonOperator.LIKE);
}

propertyCondition.setValue(instancePropertyValue);
}
else
{
Expand All @@ -225,8 +241,6 @@ public void setMatchProperties(InstanceProperties matchProperties,
{
propertyCondition.setOperator(PropertyComparisonOperator.EQ);
}

propertyCondition.setValue(instancePropertyValue);
}
}

Expand Down Expand Up @@ -280,7 +294,6 @@ private PropertyCondition getEffectiveTimePropertyCondition(MatchCriteria matchC
}



/**
* Step through the hierarchy of properties, building out the nested clauses of the search query.
*
Expand Down Expand Up @@ -427,7 +440,6 @@ private String getNestedPropertyComparisonClause(String prop
}
else
{

String sqlClause = " (";

if (topLevelPropertyName == null)
Expand All @@ -438,17 +450,16 @@ private String getNestedPropertyComparisonClause(String prop
{
sqlClause = sqlClause + RepositoryColumn.ATTRIBUTE_NAME.getColumnName() + " = '" + topLevelPropertyName + "'";
sqlClause = sqlClause + RepositoryColumn.PROPERTY_NAME.getColumnName() + " like '%:" + leafPropertyName + "'";

}
switch (operator)
{
case EQ ->
{
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " = '" + propertyValue + "') ";
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " = '" + this.getSafeRegex(propertyValue) + "') ";
}
case NEQ ->
{
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " != '" + propertyValue + "') ";
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " != '" + this.getSafeRegex(propertyValue) + "') ";
}
case LT ->
{
Expand All @@ -468,11 +479,11 @@ private String getNestedPropertyComparisonClause(String prop
}
case LIKE ->
{
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " like '" + propertyValue + "') ";
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " ~ '" + this.getSafeRegex(propertyValue) + "') ";
}
case NOT_LIKE ->
{
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " not like '" + propertyValue + "') ";
return sqlClause + " and " + RepositoryColumn.PROPERTY_VALUE.getColumnName() + " !~ '" + this.getSafeRegex(propertyValue) + "') ";
}
case NOT_NULL ->
{
Expand All @@ -487,11 +498,11 @@ private String getNestedPropertyComparisonClause(String prop
{
case EQ ->
{
return " (" + propertyColumn + " = '" + propertyValue + "') ";
return " (" + propertyColumn + " = '" + this.getSafeRegex(propertyValue) + "') ";
}
case NEQ ->
{
return " (" + propertyColumn + " != '" + propertyValue + "') ";
return " (" + propertyColumn + " != '" + this.getSafeRegex(propertyValue) + "') ";
}
case LT ->
{
Expand Down Expand Up @@ -519,11 +530,11 @@ private String getNestedPropertyComparisonClause(String prop
}
case LIKE ->
{
return " (" + propertyColumn + " like '" + propertyValue + "') ";
return " (" + propertyColumn + " like '" + this.getSafeRegex(propertyValue) + "') ";
}
case NOT_LIKE ->
{
return " (" + propertyColumn + " not like '" + propertyValue + "') ";
return " (" + propertyColumn + " not like '" + this.getSafeRegex(propertyValue) + "') ";
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ public enum PostgresErrorCode implements ExceptionMessageSet
"The search method is unable to match the supplied property with the supplied operator.",
"Correct the values supplied on the search so that single values are supplied with single value operators such as 'Equal' and multiple values are supplied on multi-value operators such as 'In'."),

/**
* POSTGRES-REPOSITORY-CONNECTOR-400-003 - The search string {0} is taking over 500 ms to run against a simple string
*/
BAD_REGEX(400,"POSTGRES-REPOSITORY-CONNECTOR-400-003",
"The search string {0} is taking over 500 ms to run against a simple string",
"An invalid parameter exception is returned to the caller since the server does not accept the request.",
"Simplify the search expression and retry the request."),

/**
* POSTGRES-REPOSITORY-CONNECTOR-500-001 - The {0} postgreSQL connector received an unexpected exception {1} during method {2}; the error message was: {3}
*/
Expand Down
Loading

0 comments on commit 227dba4

Please sign in to comment.