You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "vidakovic (via GitHub)" <gi...@apache.org> on 2023/01/26 14:03:53 UTC
[GitHub] [fineract] vidakovic opened a new pull request, #2932: FINERACT-1870: Run reports fix for 1.8.x
vidakovic opened a new pull request, #2932:
URL: https://github.com/apache/fineract/pull/2932
Run report fix for 1.8.x
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on pull request #2932: FINERACT-1870: Run reports fix for 1.8.x
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #2932:
URL: https://github.com/apache/fineract/pull/2932#issuecomment-1434202064
would you mind to write a test to cover the fixed behaviour?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2932: FINERACT-1870: Run reports fix for 1.8.x
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #2932:
URL: https://github.com/apache/fineract/pull/2932#discussion_r1107319836
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java:
##########
@@ -37,23 +38,32 @@ private SQLInjectionValidator() {
private static final String SQL_PATTERN = "[a-zA-Z_=,\\-'!><.?\"`% ()0-9*\n\r]*";
+ // TODO: see here https://rails-sqli.org for and
+ // https://larrysteinle.com/2011/02/20/use-regular-expressions-to-detect-sql-code-injection more examples
+ private static final List<String> INJECTION_PATTERNS = List.of("(?i).*[or|and]\s*[\"']?-1[\"']?\\s*(-*).*",
+ "(?i).*\\s+[\"']?(\\d+)[\"']?\\s*=\\s*[\"']?(\\1)[\"']?\\s*(-*).*");
+
public static void validateSQLInput(final String sqlSearch) {
if (StringUtils.isBlank(sqlSearch)) {
return;
}
+
+ // TODO: this should be replaced by INJECTION_PATTERNS
Review Comment:
Is the TODO for future?
Should we replace it now?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic commented on a diff in pull request #2932: FINERACT-1870: Run reports fix for 1.8.x
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #2932:
URL: https://github.com/apache/fineract/pull/2932#discussion_r1107337135
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java:
##########
@@ -37,23 +38,32 @@ private SQLInjectionValidator() {
private static final String SQL_PATTERN = "[a-zA-Z_=,\\-'!><.?\"`% ()0-9*\n\r]*";
+ // TODO: see here https://rails-sqli.org for and
+ // https://larrysteinle.com/2011/02/20/use-regular-expressions-to-detect-sql-code-injection more examples
+ private static final List<String> INJECTION_PATTERNS = List.of("(?i).*[or|and]\s*[\"']?-1[\"']?\\s*(-*).*",
+ "(?i).*\\s+[\"']?(\\d+)[\"']?\\s*=\\s*[\"']?(\\1)[\"']?\\s*(-*).*");
+
public static void validateSQLInput(final String sqlSearch) {
if (StringUtils.isBlank(sqlSearch)) {
return;
}
+
+ // TODO: this should be replaced by INJECTION_PATTERNS
Review Comment:
Well... maybe I should remove it here. This a maintenance release, so changes should be as minimal as possible. But for the upcoming upstream develop PR (not there yet) we could discuss this a bit more. There are definitely more cases to cover.
Here I am just trying to do the necessary to fix the issue. So, if you are ok with that then let's move the discussion to upstream develop and leave things as they are here. If you want I can remove the TODOs.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vidakovic merged pull request #2932: FINERACT-1870: Run reports fix for 1.8.x
Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic merged PR #2932:
URL: https://github.com/apache/fineract/pull/2932
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org