adamsaghy commented on code in PR #2624:
URL: https://github.com/apache/fineract/pull/2624#discussion_r983265643
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java:
##########
@@ -60,99 +60,72 @@ public static void validateSQLInput(final String sqlSearch)
{
}
}
- // Removing the space before and after '=' operator
- // String s = " \" OR 1 = 1"; For the cases like this
- boolean injectionFound = false;
- String inputSqlString = lowerCaseSQL;
- while (inputSqlString.indexOf(" =") > 0) { // Don't remove space before
- // = operator
- inputSqlString = inputSqlString.replaceAll(" =", "=");
- }
+ patternMatchSqlInjection(sqlSearch, lowerCaseSQL);
+ }
- while (inputSqlString.indexOf("= ") > 0) { // Don't remove space after
=
- // operator
- inputSqlString = inputSqlString.replaceAll("= ", "=");
+ public static void validateAdhocQuery(final String sqlSearch) {
+ if (StringUtils.isBlank(sqlSearch)) {
+ return;
}
-
- StringTokenizer tokenizer = new StringTokenizer(inputSqlString, " ");
- while (tokenizer.hasMoreTokens()) {
- String token = tokenizer.nextToken().trim();
- if (token.equals("'")) {
- if (tokenizer.hasMoreElements()) {
- String nextToken = tokenizer.nextToken().trim();
- if (!nextToken.equals("'")) {
- injectionFound = true;
- break;
- }
- } else {
- injectionFound = true;
- break;
- }
- }
- if (token.equals("\"")) {
- if (tokenizer.hasMoreElements()) {
- String nextToken = tokenizer.nextToken().trim();
- if (!nextToken.equals("\"")) {
- injectionFound = true;
- break;
- }
- } else {
- injectionFound = true;
- break;
- }
- } else if (token.indexOf('=') > 0) {
- StringTokenizer operatorToken = new StringTokenizer(token,
"=");
- String operand = operatorToken.nextToken().trim();
- if (!operatorToken.hasMoreTokens()) {
- injectionFound = true;
- break;
- }
- String value = operatorToken.nextToken().trim();
- if (operand.equals(value)) {
- injectionFound = true;
- break;
- }
+ String lowerCaseSQL = sqlSearch.toLowerCase().trim();
+ for (String ddl : DDL_COMMANDS) {
+ if (lowerCaseSQL.startsWith(ddl)) {
+ throw new SQLInjectionException();
}
}
- if (injectionFound) {
- throw new SQLInjectionException();
- }
- Pattern pattern = Pattern.compile(SQL_PATTERN);
- Matcher matcher = pattern.matcher(sqlSearch);
- if (!matcher.matches()) {
- throw new SQLInjectionException();
+ for (String comments : COMMENTS) {
+ if (lowerCaseSQL.contains(comments)) {
+ throw new SQLInjectionException();
+ }
}
+
+ // Removing the space before and after '=' operator
+ // String s = " \" OR 1 = 1"; For the cases like this
+ patternMatchSqlInjection(sqlSearch, lowerCaseSQL);
}
- public static void validateAdhocQuery(final String sqlSearch) {
+ public static void validateDynamicQuery(final String sqlSearch) {
if (StringUtils.isBlank(sqlSearch)) {
return;
}
- String lowerCaseSQL = sqlSearch.toLowerCase().trim();
+
+ String lowerCaseSQL = sqlSearch.toLowerCase();
for (String ddl : DDL_COMMANDS) {
- if (lowerCaseSQL.startsWith(ddl)) {
+ if (ddl.equals(lowerCaseSQL)) {
Review Comment:
I am not sure the "equals" are enough. I think the values cannot contains
any of the restricted commands.
e.g.: what if the "sqlSearch" value is ` 1; DROP TABLE m_loan; SELECT ` ?
--
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]
