You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/07/06 22:20:19 UTC

[GitHub] [fineract] thesmallstar opened a new pull request #1159: FINERACT-1006 Fixes: NullPointerException at SQLInjectionValidator.va…

thesmallstar opened a new pull request #1159:
URL: https://github.com/apache/fineract/pull/1159


   This happened due to 
   ```
     if (searchParameters.isOrderByRequested()) {
                   sqlBuilder.append(" order by ").append(searchParameters.getOrderBy()).append(' ').append(searchParameters.getSortOrder());
                   this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy(),
                           searchParameters.getSortOrder());
               }
   ```
   -> here, we incorrectly checked if OrderBY was not null but did not check if sortorder was "not null".
   
   Instead of fixing it here, it is better to fix it in the called function (IMO).
   Since the function SQLvalidateinput can be called either from columnValidator or independently, I have added check for not null at both the places.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on a change in pull request #1159: FINERACT-1006 Fixes: NullPointerException at SQLInjectionValidator.va…

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1159:
URL: https://github.com/apache/fineract/pull/1159#discussion_r453972511



##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
##########
@@ -91,6 +92,9 @@ private void validateColumn(Map<String, Set<String>> tableColumnMap) {
 
     public void validateSqlInjection(String schema, String... conditions) {
         for (String condition : conditions) {
+            if (!StringUtils.isNotBlank(condition)) {

Review comment:
       Why the "double negation" and not just if isBlank() ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger merged pull request #1159: FINERACT-1006 Fixes: NullPointerException at SQLInjectionValidator.va…

Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #1159:
URL: https://github.com/apache/fineract/pull/1159


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] awasum commented on pull request #1159: FINERACT-1006 Fixes: NullPointerException at SQLInjectionValidator.va…

Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1159:
URL: https://github.com/apache/fineract/pull/1159#issuecomment-660170946


   This is actually a fix for https://issues.apache.org/jira/browse/FINERACT-1066 as @thesmallstar pointed out on JIRA


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] thesmallstar commented on a change in pull request #1159: FINERACT-1006 Fixes: NullPointerException at SQLInjectionValidator.va…

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on a change in pull request #1159:
URL: https://github.com/apache/fineract/pull/1159#discussion_r453973699



##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
##########
@@ -91,6 +92,9 @@ private void validateColumn(Map<String, Set<String>> tableColumnMap) {
 
     public void validateSqlInjection(String schema, String... conditions) {
         for (String condition : conditions) {
+            if (!StringUtils.isNotBlank(condition)) {

Review comment:
       Opps, updating.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org