sql injections fixes
Project: http://git-wip-us.apache.org/repos/asf/incubator-fineract/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-fineract/commit/b7a5e67b Tree: http://git-wip-us.apache.org/repos/asf/incubator-fineract/tree/b7a5e67b Diff: http://git-wip-us.apache.org/repos/asf/incubator-fineract/diff/b7a5e67b Branch: refs/heads/develop Commit: b7a5e67b9e5d7d9caae77b4e8972bcdfc2c3bec2 Parents: 0c65895 Author: nazeer1100126 <[email protected]> Authored: Mon May 8 18:03:47 2017 +0530 Committer: nazeer1100126 <[email protected]> Committed: Mon May 8 18:03:47 2017 +0530 ---------------------------------------------------------------------- ...visioningEntriesReadPlatformServiceImpl.java | 5 +- .../service/AuditReadPlatformServiceImpl.java | 26 ++- ...yDatatableChecksReadPlatformServiceImpl.java | 13 +- .../ReadWriteNonCoreDataServiceImpl.java | 12 +- .../security/utils/ColumnValidator.java | 215 +++++++++++++++++++ .../security/utils/SQLInjectionException.java | 29 +++ .../security/utils/SQLInjectionValidator.java | 117 ++++++++++ .../survey/service/ReadSurveyServiceImpl.java | 12 +- .../MixTaxonomyReadPlatformServiceImpl.java | 4 +- .../NamespaceReadPlatformServiceImpl.java | 8 +- .../service/StaffReadPlatformServiceImpl.java | 67 +++--- ...TellerManagementReadPlatformServiceImpl.java | 23 +- ...ilsReadPlatformServiceJpaRepositoryImpl.java | 8 +- .../service/ChargeReadPlatformServiceImpl.java | 4 +- .../service/ClientReadPlatformServiceImpl.java | 48 +++-- .../service/CenterReadPlatformServiceImpl.java | 44 ++-- .../service/GroupReadPlatformServiceImpl.java | 46 ++-- .../service/LoanReadPlatformServiceImpl.java | 8 +- .../LoanProductReadPlatformServiceImpl.java | 10 +- .../service/MeetingReadPlatformServiceImpl.java | 3 +- .../service/NoteReadPlatformServiceImpl.java | 25 ++- ...ngsAccountChargeReadPlatformServiceImpl.java | 13 +- .../SavingsAccountReadPlatformServiceImpl.java | 6 +- .../SavingsProductReadPlatformServiceImpl.java | 4 +- 24 files changed, 611 insertions(+), 139 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesReadPlatformServiceImpl.java index 3608740..7d896a5 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/provisioning/service/ProvisioningEntriesReadPlatformServiceImpl.java @@ -242,10 +242,11 @@ public class ProvisioningEntriesReadPlatformServiceImpl implements ProvisioningE @Override public ProvisioningEntryData retrieveProvisioningEntryData(String date) { ProvisioningEntryDataMapper mapper1 = new ProvisioningEntryDataMapper(); - final String sql1 = "select " + mapper1.getSchema() + " where entry.created_date like " + "'" + date + "%'"; + date = date+"%"; + final String sql1 = "select " + mapper1.getSchema() + " where entry.created_date like ? "; ProvisioningEntryData data = null; try { - data = this.jdbcTemplate.queryForObject(sql1, mapper1, new Object[] {}); + data = this.jdbcTemplate.queryForObject(sql1, mapper1, new Object[] {date}); } catch (EmptyResultDataAccessException e) { } http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java index d754371..1315055 100755 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java @@ -41,6 +41,7 @@ import org.apache.fineract.infrastructure.core.service.Page; import org.apache.fineract.infrastructure.core.service.PaginationHelper; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; import org.apache.fineract.organisation.office.data.OfficeData; import org.apache.fineract.organisation.office.service.OfficeReadPlatformService; import org.apache.fineract.organisation.staff.data.StaffData; @@ -91,6 +92,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { private final PaginationParametersDataValidator paginationParametersDataValidator; private final SavingsProductReadPlatformService savingsProductReadPlatformService; private final DepositProductReadPlatformService depositProductReadPlatformService; + private final ColumnValidator columnValidator; @Autowired public AuditReadPlatformServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource, @@ -99,7 +101,8 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { final LoanProductReadPlatformService loanProductReadPlatformService, final StaffReadPlatformService staffReadPlatformService, final PaginationParametersDataValidator paginationParametersDataValidator, final SavingsProductReadPlatformService savingsProductReadPlatformService, - final DepositProductReadPlatformService depositProductReadPlatformService) { + final DepositProductReadPlatformService depositProductReadPlatformService, + final ColumnValidator columnValidator) { this.context = context; this.jdbcTemplate = new JdbcTemplate(dataSource); this.fromApiJsonHelper = fromApiJsonHelper; @@ -111,6 +114,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { this.paginationParametersDataValidator = paginationParametersDataValidator; this.savingsProductReadPlatformService = savingsProductReadPlatformService; this.depositProductReadPlatformService = depositProductReadPlatformService; + this.columnValidator = columnValidator; } private static final class AuditMapper implements RowMapper<AuditData> { @@ -190,7 +194,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { } updatedExtraCriteria += " order by aud.id DESC limit " + PaginationParameters.getCheckedLimit(null); - return retrieveEntries("audit", updatedExtraCriteria, includeJson); + return retrieveEntries("audit", updatedExtraCriteria, includeJson, StringUtils.isNotBlank(extraCriteria)); } @Override @@ -203,7 +207,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { String updatedExtraCriteria = ""; if (StringUtils.isNotBlank(extraCriteria)) { - updatedExtraCriteria = " where (" + extraCriteria + ")"; + updatedExtraCriteria = " where (" + extraCriteria + ")"; } final AuditMapper rm = new AuditMapper(); @@ -211,7 +215,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { sqlBuilder.append("select SQL_CALC_FOUND_ROWS "); sqlBuilder.append(rm.schema(includeJson, hierarchy)); sqlBuilder.append(' ').append(updatedExtraCriteria); - + this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria); if (parameters.isOrderByRequested()) { sqlBuilder.append(' ').append(parameters.orderBySql()); } else { @@ -240,10 +244,10 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { updatedExtraCriteria += " group by aud.id order by aud.id"; - return retrieveEntries("makerchecker", updatedExtraCriteria, includeJson); + return retrieveEntries("makerchecker", updatedExtraCriteria, includeJson, StringUtils.isNotBlank(extraCriteria)); } - public Collection<AuditData> retrieveEntries(final String useType, final String extraCriteria, final boolean includeJson) { + public Collection<AuditData> retrieveEntries(final String useType, final String extraCriteria, final boolean includeJson, boolean isExtraCritereaIncluded) { if (!(useType.equals("audit") || useType.equals("makerchecker"))) { throw new PlatformDataIntegrityException( "error.msg.invalid.auditSearchTemplate.useType", "Invalid Audit Search Template UseType: " + useType); } @@ -253,7 +257,7 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { final AuditMapper rm = new AuditMapper(); String sql = "select " + rm.schema(includeJson, hierarchy); - + Boolean isLimitedChecker = false; if (useType.equals("makerchecker")) { if (currentUser.hasNotPermissionForAnyOf("ALL_FUNCTIONS", "CHECKER_SUPER_USER")) { @@ -267,7 +271,9 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { + " join m_appuser_role ur on ur.role_id = r.id and ur.appuser_id = " + currentUser.getId(); } sql += extraCriteria; - + if(isExtraCritereaIncluded){ + this.columnValidator.validateSqlInjection(sql, extraCriteria); + } logger.info("sql: " + sql); return this.jdbcTemplate.query(sql, rm, new Object[] {}); @@ -281,9 +287,9 @@ public class AuditReadPlatformServiceImpl implements AuditReadPlatformService { final AuditMapper rm = new AuditMapper(); - final String sql = "select " + rm.schema(true, hierarchy) + " where aud.id = " + auditId; + final String sql = "select " + rm.schema(true, hierarchy) + " where aud.id = ? "; - final AuditData auditResult = this.jdbcTemplate.queryForObject(sql, rm, new Object[] {}); + final AuditData auditResult = this.jdbcTemplate.queryForObject(sql, rm, new Object[] {auditId}); return replaceIdsOnAuditData(auditResult); } http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/EntityDatatableChecksReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/EntityDatatableChecksReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/EntityDatatableChecksReadPlatformServiceImpl.java index 31f58a2..56b6a1c 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/EntityDatatableChecksReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/EntityDatatableChecksReadPlatformServiceImpl.java @@ -86,17 +86,20 @@ public class EntityDatatableChecksReadPlatformServiceImpl implements EntityDatat if (status != null || entity != null || productId != null) { sqlBuilder.append(" where "); } - + List<Object> paramList = new ArrayList<>(); if (status != null) { - sqlBuilder.append(" status_enum = " + status); + sqlBuilder.append(" status_enum = ? "); + paramList.add(status); } if (entity != null) { - sqlBuilder.append(" and t.application_table_name = '" + entity + "'"); + sqlBuilder.append(" and t.application_table_name = ? "); + paramList.add(entity); } if (productId != null) { - sqlBuilder.append(" and t.product_id = " + productId); + sqlBuilder.append(" and t.product_id = ? "); + paramList.add(productId); } if (searchParameters.isLimited()) { sqlBuilder.append(" limit ").append(searchParameters.getLimit()); @@ -105,7 +108,7 @@ public class EntityDatatableChecksReadPlatformServiceImpl implements EntityDatat } } final String sqlCountRows = "SELECT FOUND_ROWS()"; - return this.paginationHelper.fetchPage(jdbcTemplate, sqlCountRows, sqlBuilder.toString(), new Object[] {}, + return this.paginationHelper.fetchPage(jdbcTemplate, sqlCountRows, sqlBuilder.toString(),paramList.toArray(), entityDataTableChecksMapper); } http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java index 0bcb144..6b91255 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java @@ -49,6 +49,7 @@ import org.apache.fineract.infrastructure.dataqueries.exception.DatatableEntryRe import org.apache.fineract.infrastructure.dataqueries.exception.DatatableNotFoundException; import org.apache.fineract.infrastructure.dataqueries.exception.DatatableSystemErrorException; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.useradministration.domain.AppUser; import org.joda.time.LocalDate; import org.joda.time.LocalDateTime; @@ -134,6 +135,7 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ if (appTable == null) { andClause = ""; } else { + SQLInjectionValidator.validateSQLInput(appTable); andClause = " and application_table_name = '" + appTable + "'"; } @@ -164,6 +166,7 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ public DatatableData retrieveDatatable(final String datatable) { // PERMITTED datatables + SQLInjectionValidator.validateSQLInput(datatable); final String sql = "select application_table_name, registered_table_name" + " from x_registered_table " + " where exists" + " (select 'f'" + " from m_appuser_role ur " + " join m_role r on r.id = ur.role_id" + " left join m_role_permission rp on rp.role_id = r.id" + " left join m_permission p on p.id = rp.permission_id" @@ -1161,7 +1164,9 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ // id only used for reading a specific entry in a one to many datatable // (when updating) if (id == null) { - sql = sql + "select * from `" + dataTableName + "` where " + getFKField(appTable) + " = " + appTableId; + String whereClause = getFKField(appTable) + " = " + appTableId; + SQLInjectionValidator.validateSQLInput(whereClause); + sql = sql + "select * from `" + dataTableName + "` where " + whereClause; } else { sql = sql + "select * from `" + dataTableName + "` where id = " + id; } @@ -1185,7 +1190,9 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ // id only used for reading a specific entry in a one to many datatable // (when updating) if (id == null) { - sql = sql + "select * from `" + dataTableName + "` where " + getFKField(appTable) + " = " + appTableId; + String whereClause = getFKField(appTable) + " = " + appTableId; + SQLInjectionValidator.validateSQLInput(whereClause); + sql = sql + "select * from `" + dataTableName + "` where " + whereClause; } else { sql = sql + "select * from `" + dataTableName + "` where id = " + id; } @@ -1333,6 +1340,7 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ } private String queryForApplicationTableName(final String datatable) { + SQLInjectionValidator.validateSQLInput(datatable); final String sql = "SELECT application_table_name FROM x_registered_table where registered_table_name = '" + datatable + "'"; final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(sql); http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java new file mode 100644 index 0000000..724dd42 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java @@ -0,0 +1,215 @@ +/** + * Licensed to the Apache ftware Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.infrastructure.security.utils; + +import java.sql.DatabaseMetaData; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.fineract.infrastructure.core.service.RoutingDataSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.stereotype.Component; + +@Component +public class ColumnValidator { + + private final JdbcTemplate jdbcTemplate; + + @Autowired + public ColumnValidator(final RoutingDataSource dataSource) { + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + + private void validateColumn(Map<String, Set<String>> tableColumnMap) { + try { + DatabaseMetaData dbMetaData = this.jdbcTemplate.getDataSource() + .getConnection().getMetaData(); + ResultSet resultSet = null; + for (HashMap.Entry<String, Set<String>> entry : tableColumnMap + .entrySet()) { + Set<String> columns = entry.getValue(); + resultSet = dbMetaData.getColumns(null, null, entry.getKey(), + null); + Set<String> tableColumns = getTableColumns(resultSet); + if (columns.size() > 0 && tableColumns.size() == 0) { + throw new SQLInjectionException(); + } + for (String requestedColumn : columns) { + if (!tableColumns.contains(requestedColumn)) { + throw new SQLInjectionException(); + } + } + } + } catch (SQLException e) { + throw new SQLInjectionException(); + } + } + + private Set<String> getTableColumns(final ResultSet rs) { + Set<String> columns = new HashSet<>(); + try { + while (rs.next()) { + columns.add(rs.getString("column_name")); + } + } catch (SQLException e) { + e.printStackTrace(); + } + return columns; + } + + public void validateSqlInjection(String schema, String condition) { + SQLInjectionValidator.validateSQLInput(condition); + List<String> operator = new ArrayList<>(Arrays.asList("=", ">", "<", + "> =", "< =", "! =", "!=", ">=", "<=")); + condition = condition.trim().replace("( ", "(").replace(" )", ")") + .toLowerCase(); + for (String op : operator) { + condition = replaceAll(condition, op).replaceAll(" +", " "); + } + Set<String> operands = getOperand(condition); + schema = schema.trim().replaceAll(" +", " ").toLowerCase(); + Map<String, Set<String>> tableColumnAliasMap = getTableColumnAliasMap(operands); + Map<String, Set<String>> tableColumnMap = getTableColumnMap(schema, + tableColumnAliasMap); + validateColumn(tableColumnMap); + } + + private static Map<String, Set<String>> getTableColumnMap(String schema, + Map<String, Set<String>> tableColumnAliasMap) { + Map<String, Set<String>> tableColumnMap = new HashMap<>(); + schema = schema.substring(schema.indexOf("from")); + for (String alias : tableColumnAliasMap.keySet()) { + int index = schema.indexOf(" " + alias + " "); + if (index > -1) { + int startPos = 0; + startPos = schema.substring(0, index - 1).lastIndexOf(' ', + index); + Set<String> columns = tableColumnAliasMap.get(alias); + tableColumnMap.put(schema.substring(startPos, index).trim(), + columns); + } else { + throw new SQLInjectionException(); + } + } + return tableColumnMap; + } + + private static Map<String, Set<String>> getTableColumnAliasMap( + Set<String> operands) { + Map<String, Set<String>> tableColumnMap = new HashMap<>(); + for (String operand : operands) { + String[] tableColumn = operand.split("\\."); + if (tableColumn.length == 2) { + if (tableColumnMap.containsKey(tableColumn[0])) { + Set<String> columns = tableColumnMap.get(tableColumn[0]); + columns.add(tableColumn[1]); + } else { + Set<String> columns = new HashSet<>(); + columns.add(tableColumn[1]); + tableColumnMap.put(tableColumn[0], columns); + } + } else { + throw new SQLInjectionException(); + } + } + return tableColumnMap; + } + + private static Set<String> getOperand(String condition) { + Set<String> operandList = new HashSet<>(); + List<String> operatorList = new ArrayList<>(Arrays.asList("!=", "=", + ">", "<", " like ", " between ", " in ", " in(", " is ", + " is not ", " equals ", " not equals ")); + for (String op : operatorList) { + int startIndex = 0; + do { + int index = condition.indexOf(op, startIndex); + if (index > -1) { + char currentChar = condition.charAt(index - 1); + if (op.equals("=")) { + if (!((currentChar + "").equals("!") + || (currentChar + "").equals(">") || (currentChar + "") + .equals("<"))) { + operandList.add(getOperand(condition, index, + currentChar)); + } + } else { + operandList.add(getOperand(condition, index, + currentChar)); + } + + startIndex = index + op.length(); + } + + } while (condition.indexOf(op, startIndex) > -1); + } + return operandList; + } + + private static String getOperand(String condition, int index, + char currentChar) { + int startPos = 0; + if ((currentChar + "").equals(" ")) { + startPos = condition.substring(0, index - 1) + .lastIndexOf(' ', index); + } else { + startPos = condition.substring(0, index).lastIndexOf(' ', index); + } + String a = condition.substring(startPos == -1 ? 0 : startPos, index); + return a.trim().replace("(", "").replace(")", ""); + } + + private static String replaceAll(String condition, String op) { + int startIndex = 0; + do { + int index = condition.indexOf(op, startIndex); + if (index > -1) { + if (op.equals("=")) { + if (!((condition.charAt(index - 1) + "").equals("!") + || (condition.charAt(index - 1) + "").equals(">") || (condition + .charAt(index - 1) + "").equals("<"))) { + condition = condition.replace(op, " " + op + " "); + return condition; + } + startIndex = index + 2 + op.length(); + + } else if (op.equals("< =") || op.equals("> =") + || op.equals("! =")) { + condition = condition.replace(op, op.replace(" ", "")); + return condition; + } else { + condition = condition.replace(op, " " + op + " "); + return condition; + } + } else { + return condition; + } + + } while (condition.indexOf(op, startIndex) > -1); + return condition; + } +} http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionException.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionException.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionException.java new file mode 100644 index 0000000..e87003e --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionException.java @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.infrastructure.security.utils; + +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; + +public class SQLInjectionException extends PlatformApiDataValidationException { + + public SQLInjectionException() { + super("error.msg.found.sql.injection", "Unexpected SQL Commands found", null); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java new file mode 100644 index 0000000..03785e0 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLInjectionValidator.java @@ -0,0 +1,117 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.infrastructure.security.utils; + +import java.util.StringTokenizer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class SQLInjectionValidator { + + private final static String[] DDL_COMMANDS = { "create", "drop", "alter", "truncate", "comment" }; + + private final static String[] DML_COMMANDS = { "select", "insert", "update", "delete", "merge", "upsert", "call" }; + + private final static String[] COMMENTS = { "--", "({", "/*", "#" }; + + private final static String SQL_PATTERN = "[a-zA-Z_=,'!><.?\"`% ()0-9]*"; + + public final static void validateSQLInput(final String sqlSearch) { + String lowerCaseSQL = sqlSearch.toLowerCase(); + for (String ddl : DDL_COMMANDS) { + if (lowerCaseSQL.contains(ddl)) { + throw new SQLInjectionException(); + } + } + + for (String dml : DML_COMMANDS) { + if (lowerCaseSQL.contains(dml)) { + 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 + boolean injectionFound = false; + String inputSqlString = lowerCaseSQL; + while (inputSqlString.indexOf(" =") > 0) { //Don't remove space before = operator + inputSqlString = inputSqlString.replaceAll(" =", "="); + } + + while (inputSqlString.indexOf("= ") > 0) { //Don't remove space after = operator + inputSqlString = inputSqlString.replaceAll("= ", "="); + } + + 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; + } + } + } + if (injectionFound) { + throw new SQLInjectionException(); + } + + Pattern pattern = Pattern.compile(SQL_PATTERN); + Matcher matcher = pattern.matcher(sqlSearch); + if (!matcher.matches()) { + throw new SQLInjectionException(); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/survey/service/ReadSurveyServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/survey/service/ReadSurveyServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/survey/service/ReadSurveyServiceImpl.java index 9844006..fe8f87c 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/survey/service/ReadSurveyServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/survey/service/ReadSurveyServiceImpl.java @@ -31,6 +31,7 @@ import org.apache.fineract.infrastructure.dataqueries.data.ResultsetColumnHeader import org.apache.fineract.infrastructure.dataqueries.service.GenericDataService; import org.apache.fineract.infrastructure.dataqueries.service.ReadWriteNonCoreDataService; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.infrastructure.survey.data.ClientScoresOverview; import org.apache.fineract.infrastructure.survey.data.LikelihoodStatus; import org.apache.fineract.infrastructure.survey.data.SurveyDataTableData; @@ -96,6 +97,7 @@ public class ReadSurveyServiceImpl implements ReadSurveyService { @Override public SurveyDataTableData retrieveSurvey(String surveyName) { + SQLInjectionValidator.validateSQLInput(surveyName); final String sql = "select cf.enabled, application_table_name, registered_table_name" + " from x_registered_table " + " left join c_configuration cf on x_registered_table.registered_table_name = cf.name " + " where exists" + " (select 'f'" + " from m_appuser_role ur " + " join m_role r on r.id = ur.role_id" @@ -125,13 +127,13 @@ public class ReadSurveyServiceImpl implements ReadSurveyService { @Override public List<ClientScoresOverview> retrieveClientSurveyScoreOverview(String surveyName, Long clientId) { - final String sql = "SELECT tz.id, lkh.name, lkh.code, poverty_line, tz.date, tz.score FROM " + surveyName + " tz" - + " JOIN ppi_likelihoods_ppi lkp on lkp.ppi_name = '" + surveyName + "' AND enabled = '" + LikelihoodStatus.ENABLED - + "' JOIN ppi_scores sc on score_from <= tz.score AND score_to >=tz.score" + final String sql = "SELECT tz.id, lkh.name, lkh.code, poverty_line, tz.date, tz.score FROM ? tz" + + " JOIN ppi_likelihoods_ppi lkp on lkp.ppi_name = ? AND enabled = ? " + + " JOIN ppi_scores sc on score_from <= tz.score AND score_to >=tz.score" + " JOIN ppi_poverty_line pvl on pvl.likelihood_ppi_id = lkp.id AND pvl.score_id = sc.id" - + " JOIN ppi_likelihoods lkh on lkh.id = lkp.likelihood_id " + " WHERE client_id = " + clientId; + + " JOIN ppi_likelihoods lkh on lkh.id = lkp.likelihood_id " + " WHERE client_id = ? "; - final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(sql); + final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(sql, new Object[] { surveyName, surveyName, LikelihoodStatus.ENABLED, clientId }); List<ClientScoresOverview> scoresOverviews = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/mix/service/MixTaxonomyReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/mix/service/MixTaxonomyReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/mix/service/MixTaxonomyReadPlatformServiceImpl.java index 25d2124..8c9f867 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/mix/service/MixTaxonomyReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/mix/service/MixTaxonomyReadPlatformServiceImpl.java @@ -70,7 +70,7 @@ public class MixTaxonomyReadPlatformServiceImpl implements MixTaxonomyReadPlatfo @Override public MixTaxonomyData retrieveOne(final Long id) { - final String sql = "select " + this.mixTaxonomyMapper.schema() + " where tx.id =" + id; - return this.jdbcTemplate.queryForObject(sql, this.mixTaxonomyMapper); + final String sql = "select " + this.mixTaxonomyMapper.schema() + " where tx.id = ? "; + return this.jdbcTemplate.queryForObject(sql, this.mixTaxonomyMapper, new Object[] { id }); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/mix/service/NamespaceReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/mix/service/NamespaceReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/mix/service/NamespaceReadPlatformServiceImpl.java index 3cfd26b..b0dfe82 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/mix/service/NamespaceReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/mix/service/NamespaceReadPlatformServiceImpl.java @@ -58,15 +58,15 @@ public class NamespaceReadPlatformServiceImpl implements NamespaceReadPlatformSe @Override public NamespaceData retrieveNamespaceById(final Long id) { - final String sql = this.namespaceMapper.schema() + " where id=" + id; + final String sql = this.namespaceMapper.schema() + " where id= ? "; - return this.jdbcTemplate.queryForObject(sql, this.namespaceMapper); + return this.jdbcTemplate.queryForObject(sql, this.namespaceMapper, new Object[] { id }); } @Override public NamespaceData retrieveNamespaceByPrefix(final String prefix) { - final String sql = this.namespaceMapper.schema() + " where prefix='" + prefix + "'"; + final String sql = this.namespaceMapper.schema() + " where prefix = ? "; - return this.jdbcTemplate.queryForObject(sql, this.namespaceMapper); + return this.jdbcTemplate.queryForObject(sql, this.namespaceMapper, new Object[] { prefix }); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java index 73f648e..da2cea7 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/organisation/staff/service/StaffReadPlatformServiceImpl.java @@ -30,6 +30,8 @@ import org.apache.fineract.infrastructure.core.domain.JdbcSupport; import org.apache.fineract.infrastructure.core.exception.UnrecognizedQueryParamException; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.organisation.staff.data.StaffData; import org.apache.fineract.organisation.staff.exception.StaffNotFoundException; import org.apache.fineract.portfolio.client.domain.ClientStatus; @@ -49,11 +51,14 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { private final PlatformSecurityContext context; private final StaffLookupMapper lookupMapper = new StaffLookupMapper(); private final StaffInOfficeHierarchyMapper staffInOfficeHierarchyMapper = new StaffInOfficeHierarchyMapper(); + private final ColumnValidator columnValidator; @Autowired - public StaffReadPlatformServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource) { + public StaffReadPlatformServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource, + final ColumnValidator columnValidator) { this.context = context; this.jdbcTemplate = new JdbcTemplate(dataSource); + this.columnValidator = columnValidator; } private static final class StaffMapper implements RowMapper<StaffData> { @@ -157,20 +162,20 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { @Override public Collection<StaffData> retrieveAllLoanOfficersInOfficeById(final Long officeId) { - return retrieveAllStaff(" office_id=" + officeId + " and is_loan_officer=1"); + return retrieveAllStaff(" office_id = ? and is_loan_officer=1", officeId); } @Override public Collection<StaffData> retrieveAllStaffForDropdown(final Long officeId) { //adding the Authorization criteria so that a user cannot see an employee who does not belong to his office or a sub office for his office. - final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy(); + final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy()+"%"; final Long defaultOfficeId = defaultToUsersOfficeIfNull(officeId); - final String sql = "select " + this.lookupMapper.schema() + " where s.office_id = ? and s.is_active=1 and o.hierarchy like '"+ hierarchy+ "%'"; + final String sql = "select " + this.lookupMapper.schema() + " where s.office_id = ? and s.is_active=1 and o.hierarchy like ? "; - return this.jdbcTemplate.query(sql, this.lookupMapper, new Object[] { defaultOfficeId }); + return this.jdbcTemplate.query(sql, this.lookupMapper, new Object[] { defaultOfficeId, hierarchy }); } private Long defaultToUsersOfficeIfNull(final Long officeId) { @@ -185,14 +190,14 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { public StaffData retrieveStaff(final Long staffId) { //adding the Authorization criteria so that a user cannot see an employee who does not belong to his office or a sub office for his office. - final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy(); + final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy()+ "%"; try { final StaffMapper rm = new StaffMapper(); - final String sql = "select " + rm.schema() + " where s.id = ? and o.hierarchy like '"+ hierarchy+ "%'" ; + final String sql = "select " + rm.schema() + " where s.id = ? and o.hierarchy like ? " ; - return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { staffId }); + return this.jdbcTemplate.queryForObject(sql, rm, new Object[] { staffId, hierarchy }); } catch (final EmptyResultDataAccessException e) { throw new StaffNotFoundException(staffId); } @@ -202,18 +207,22 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { public Collection<StaffData> retrieveAllStaff(final String sqlSearch, final Long officeId, final boolean loanOfficersOnly, final String status) { final String extraCriteria = getStaffCriteria(sqlSearch, officeId, loanOfficersOnly, status); - return retrieveAllStaff(extraCriteria); + return retrieveAllStaff(extraCriteria, officeId); } - private Collection<StaffData> retrieveAllStaff(final String extraCriteria) { - + private Collection<StaffData> retrieveAllStaff(final String extraCriteria, Long officeId) { + final StaffMapper rm = new StaffMapper(); String sql = "select " + rm.schema(); - if (StringUtils.isNotBlank(extraCriteria)) { - sql += " where " + extraCriteria; + final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy()+"%"; + if (StringUtils.isNotBlank(extraCriteria)){ + sql += " where " + extraCriteria; } sql = sql + " order by s.lastname"; - return this.jdbcTemplate.query(sql, rm, new Object[] {}); + if(officeId==null){ + return this.jdbcTemplate.query(sql, rm, new Object[] {hierarchy }); + } + return this.jdbcTemplate.query(sql, rm, new Object[] {officeId, hierarchy }); } private String getStaffCriteria(final String sqlSearch, final Long officeId, final boolean loanOfficersOnly, final String status) { @@ -222,9 +231,11 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { if (sqlSearch != null) { extraCriteria.append(" and (").append(sqlSearch).append(")"); + final StaffMapper rm = new StaffMapper(); + this.columnValidator.validateSqlInjection(rm.schema(), sqlSearch); } if (officeId != null) { - extraCriteria.append(" and office_id = ").append(officeId).append(" "); + extraCriteria.append(" and s.office_id = ? "); } if (loanOfficersOnly) { extraCriteria.append(" and s.is_loan_officer is true "); @@ -232,21 +243,21 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { // Passing status parameter to get ACTIVE (By Default), INACTIVE or ALL // (Both active and Inactive) employees if (status.equalsIgnoreCase("active")) { - extraCriteria.append(" and is_active = 1 "); + extraCriteria.append(" and s.is_active = 1 "); } else if (status.equalsIgnoreCase("inActive")) { - extraCriteria.append(" and is_active = 0 "); + extraCriteria.append(" and s.is_active = 0 "); } else if (status.equalsIgnoreCase("all")) {} else { throw new UnrecognizedQueryParamException("status", status, new Object[] { "all", "active", "inactive" }); } //adding the Authorization criteria so that a user cannot see an employee who does not belong to his office or a sub office for his office. - final String hierarchy = this.context.authenticatedUser().getOffice().getHierarchy(); - extraCriteria.append(" and o.hierarchy like '"+ hierarchy+ "%' "); + + extraCriteria.append(" and o.hierarchy like ? "); if (StringUtils.isNotBlank(extraCriteria.toString())) { extraCriteria.delete(0, 4); } - + // remove begin four letter including a space from the string. return extraCriteria.toString(); } @@ -265,15 +276,21 @@ public class StaffReadPlatformServiceImpl implements StaffReadPlatformService { String sql = "select c.display_name as client, g.display_name as grp,l.loan_officer_id as loan, s.field_officer_id as sav"+ " from m_staff staff "+ - " left outer join m_client c on staff.id = c.staff_id AND c.status_enum < "+ ClientStatus.CLOSED.getValue() + + " left outer join m_client c on staff.id = c.staff_id AND c.status_enum < ? "+ " left outer join m_group g on staff.id = g.staff_id " + - " left outer join m_loan l on staff.id = l.loan_officer_id and l.loan_status_id < " + LoanStatus.WITHDRAWN_BY_CLIENT.getValue() + - " left outer join m_savings_account s on c.staff_id = s.field_officer_id and s.status_enum < "+ SavingsAccountStatusType.WITHDRAWN_BY_APPLICANT.getValue() + - " where staff.id = " + staffId + + " left outer join m_loan l on staff.id = l.loan_officer_id and l.loan_status_id < ? " + + " left outer join m_savings_account s on c.staff_id = s.field_officer_id and s.status_enum < ? "+ + " where staff.id = ? "+ " group by staff.id"; - List<Map<String, Object>> result = this.jdbcTemplate.queryForList(sql); + List<Map<String, Object>> result = this.jdbcTemplate.queryForList( + sql, + new Object[] { + ClientStatus.CLOSED.getValue(), + LoanStatus.WITHDRAWN_BY_CLIENT.getValue(), + SavingsAccountStatusType.WITHDRAWN_BY_APPLICANT + .getValue(), staffId }); if (result != null) { for (Map<String, Object> map : result) { if (map.get("client") != null) { http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/organisation/teller/service/TellerManagementReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/organisation/teller/service/TellerManagementReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/organisation/teller/service/TellerManagementReadPlatformServiceImpl.java index 03fc11c..7451795 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/organisation/teller/service/TellerManagementReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/organisation/teller/service/TellerManagementReadPlatformServiceImpl.java @@ -33,6 +33,7 @@ import org.apache.fineract.infrastructure.core.service.PaginationHelper; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.core.service.SearchParameters; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; import org.apache.fineract.organisation.monetary.data.CurrencyData; import org.apache.fineract.organisation.monetary.service.CurrencyReadPlatformService; import org.apache.fineract.organisation.office.data.OfficeData; @@ -70,16 +71,18 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement private final StaffReadPlatformService staffReadPlatformService; private final CurrencyReadPlatformService currencyReadPlatformService; private final PaginationHelper<CashierTransactionData> paginationHelper = new PaginationHelper<>(); + private final ColumnValidator columnValidator; @Autowired public TellerManagementReadPlatformServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource, final OfficeReadPlatformService officeReadPlatformService, StaffReadPlatformService staffReadPlatformService, - final CurrencyReadPlatformService currencyReadPlatformService) { + final CurrencyReadPlatformService currencyReadPlatformService, final ColumnValidator columnValidator) { this.context = context; this.jdbcTemplate = new JdbcTemplate(dataSource); this.officeReadPlatformService = officeReadPlatformService; this.staffReadPlatformService = staffReadPlatformService; this.currencyReadPlatformService = currencyReadPlatformService; + this.columnValidator = columnValidator; } private static final class TellerMapper implements RowMapper<TellerData> { @@ -223,11 +226,11 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement @Override public Collection<TellerData> retrieveAllTellers(final String sqlSearch, final Long officeId, final String status) { - final String extraCriteria = getTellerCriteria(sqlSearch, officeId, status); - return retrieveAllTeller(extraCriteria); + final String extraCriteria = getTellerCriteria(sqlSearch, officeId, status); + return retrieveAllTeller(extraCriteria, officeId); } - private Collection<TellerData> retrieveAllTeller(final String extraCriteria) { + private Collection<TellerData> retrieveAllTeller(final String extraCriteria, final Long officeId) { final TellerMapper tm = new TellerMapper(); String sql = "select " + tm.schema(); @@ -235,6 +238,9 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement sql += " where " + extraCriteria; } sql = sql + " order by t.teller_name"; + if(officeId!=null){ + return this.jdbcTemplate.query(sql, tm, new Object[] {officeId}); + } return this.jdbcTemplate.query(sql, tm, new Object[] {}); } @@ -244,9 +250,11 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement if (sqlSearch != null) { extraCriteria.append(" and (").append(sqlSearch).append(")"); + final TellerMapper tm = new TellerMapper(); + this.columnValidator.validateSqlInjection(tm.schema(), sqlSearch); } if (officeId != null) { - extraCriteria.append(" and office_id = ").append(officeId).append(" "); + extraCriteria.append(" and office_id = ? "); } // Passing status parameter to get ACTIVE (By Default), INACTIVE or ALL // (Both active and Inactive) employees @@ -278,7 +286,7 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement @Override public Collection<CashierData> retrieveCashiersForTellers(final String sqlSearch, final Long tellerId) { - final String extraCriteria = getTellerCriteria(sqlSearch, tellerId); + final String extraCriteria = getTellerCriteria(sqlSearch, tellerId); return fetchCashiers(extraCriteria); } @@ -288,6 +296,9 @@ public class TellerManagementReadPlatformServiceImpl implements TellerManagement if (sqlSearch != null) { extraCriteria.append(" and (").append(sqlSearch).append(")"); + final CashierMapper cm = new CashierMapper(); + this.columnValidator.validateSqlInjection(cm.schema(), sqlSearch); + } if (tellerId != null) { extraCriteria.append(" and teller_id = ").append(tellerId).append(" "); http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java index bde382a..0fc9103 100755 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/accountdetails/service/AccountDetailsReadPlatformServiceJpaRepositoryImpl.java @@ -27,6 +27,7 @@ import java.util.List; import org.apache.fineract.infrastructure.core.data.EnumOptionData; import org.apache.fineract.infrastructure.core.domain.JdbcSupport; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; import org.apache.fineract.organisation.monetary.data.CurrencyData; import org.apache.fineract.portfolio.accountdetails.data.AccountSummaryCollectionData; import org.apache.fineract.portfolio.accountdetails.data.LoanAccountSummaryData; @@ -56,13 +57,16 @@ public class AccountDetailsReadPlatformServiceJpaRepositoryImpl implements Accou private final JdbcTemplate jdbcTemplate; private final ClientReadPlatformService clientReadPlatformService; private final GroupReadPlatformService groupReadPlatformService; + private final ColumnValidator columnValidator; @Autowired public AccountDetailsReadPlatformServiceJpaRepositoryImpl(final ClientReadPlatformService clientReadPlatformService, - final RoutingDataSource dataSource, final GroupReadPlatformService groupReadPlatformService) { + final RoutingDataSource dataSource, final GroupReadPlatformService groupReadPlatformService, + final ColumnValidator columnValidator) { this.clientReadPlatformService = clientReadPlatformService; this.jdbcTemplate = new JdbcTemplate(dataSource); this.groupReadPlatformService = groupReadPlatformService; + this.columnValidator = columnValidator; } @Override @@ -119,6 +123,7 @@ public class AccountDetailsReadPlatformServiceJpaRepositoryImpl implements Accou private List<LoanAccountSummaryData> retrieveLoanAccountDetails(final String loanwhereClause, final Object[] inputs) { final LoanAccountSummaryDataMapper rm = new LoanAccountSummaryDataMapper(); final String sql = "select " + rm.loanAccountSummarySchema() + loanwhereClause; + this.columnValidator.validateSqlInjection(rm.loanAccountSummarySchema(), loanwhereClause); return this.jdbcTemplate.query(sql, rm, inputs); } @@ -129,6 +134,7 @@ public class AccountDetailsReadPlatformServiceJpaRepositoryImpl implements Accou private List<SavingsAccountSummaryData> retrieveAccountDetails(final String savingswhereClause, final Object[] inputs) { final SavingsAccountSummaryDataMapper savingsAccountSummaryDataMapper = new SavingsAccountSummaryDataMapper(); final String savingsSql = "select " + savingsAccountSummaryDataMapper.schema() + savingswhereClause; + this.columnValidator.validateSqlInjection(savingsAccountSummaryDataMapper.schema() , savingswhereClause); return this.jdbcTemplate.query(savingsSql, savingsAccountSummaryDataMapper, inputs); } http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/service/ChargeReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/service/ChargeReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/service/ChargeReadPlatformServiceImpl.java index 8d438cc..f3ca619 100755 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/service/ChargeReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/charge/service/ChargeReadPlatformServiceImpl.java @@ -102,13 +102,13 @@ public class ChargeReadPlatformServiceImpl implements ChargeReadPlatformService public Collection<ChargeData> retrieveAllChargesForCurrency(String currencyCode) { final ChargeMapper rm = new ChargeMapper(); - String sql = "select " + rm.chargeSchema() + " where c.is_deleted=false and c.currency_code='" + currencyCode + "' "; + String sql = "select " + rm.chargeSchema() + " where c.is_deleted=false and c.currency_code= ? "; sql += addInClauseToSQL_toLimitChargesMappedToOffice_ifOfficeSpecificProductsEnabled(); sql += " order by c.name "; - return this.jdbcTemplate.query(sql, rm, new Object[] {}); + return this.jdbcTemplate.query(sql, rm, new Object[] {currencyCode}); } @Override http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java index c521729..24d85c5 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/client/service/ClientReadPlatformServiceImpl.java @@ -21,6 +21,7 @@ package org.apache.fineract.portfolio.client.service; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -41,6 +42,7 @@ import org.apache.fineract.infrastructure.dataqueries.data.EntityTables; import org.apache.fineract.infrastructure.dataqueries.data.StatusEnum; import org.apache.fineract.infrastructure.dataqueries.service.EntityDatatableChecksReadService; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; import org.apache.fineract.organisation.office.data.OfficeData; import org.apache.fineract.organisation.office.service.OfficeReadPlatformService; import org.apache.fineract.organisation.staff.data.StaffData; @@ -86,6 +88,7 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService private final AddressReadPlatformService addressReadPlatformService; private final ConfigurationReadPlatformService configurationReadPlatformService; private final EntityDatatableChecksReadService entityDatatableChecksReadService; + private final ColumnValidator columnValidator; @Autowired public ClientReadPlatformServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource, @@ -94,7 +97,8 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService final SavingsProductReadPlatformService savingsProductReadPlatformService, final AddressReadPlatformService addressReadPlatformService, final ConfigurationReadPlatformService configurationReadPlatformService, - final EntityDatatableChecksReadService entityDatatableChecksReadService) { + final EntityDatatableChecksReadService entityDatatableChecksReadService, + final ColumnValidator columnValidator) { this.context = context; this.officeReadPlatformService = officeReadPlatformService; this.jdbcTemplate = new JdbcTemplate(dataSource); @@ -104,6 +108,7 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService this.addressReadPlatformService=addressReadPlatformService; this.configurationReadPlatformService=configurationReadPlatformService; this.entityDatatableChecksReadService = entityDatatableChecksReadService; + this.columnValidator = columnValidator; } @Override @@ -165,6 +170,7 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService } @Override + // @Transactional(readOnly=true) public Page<ClientData> retrieveAll(final SearchParameters searchParameters) { final String userOfficeHierarchy = this.context.officeHierarchy(); @@ -175,7 +181,7 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService // this.context.validateAccessRights(searchParameters.getHierarchy()); // underHierarchySearchString = searchParameters.getHierarchy() + "%"; // } - + List<Object> paramList = new ArrayList<>(Arrays.asList(underHierarchySearchString, underHierarchySearchString)); final StringBuilder sqlBuilder = new StringBuilder(200); sqlBuilder.append("select SQL_CALC_FOUND_ROWS "); sqlBuilder.append(this.clientMapper.schema()); @@ -183,10 +189,11 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService if(searchParameters.isSelfUser()){ sqlBuilder.append(" and c.id in (select umap.client_id from m_selfservice_user_client_mapping as umap where umap.appuser_id = ? ) "); + paramList.add(appUserID); } - final String extraCriteria = buildSqlStringFromClientCriteria(searchParameters); - + final String extraCriteria = buildSqlStringFromClientCriteria(this.clientMapper.schema(), searchParameters, paramList); + if (StringUtils.isNotBlank(extraCriteria)) { sqlBuilder.append(" and (").append(extraCriteria).append(")"); } @@ -207,14 +214,10 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService } final String sqlCountRows = "SELECT FOUND_ROWS()"; - Object[] params = new Object[] {underHierarchySearchString, underHierarchySearchString }; - if(searchParameters.isSelfUser()){ - params = new Object[] {underHierarchySearchString, underHierarchySearchString, appUserID }; - } - return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), params, this.clientMapper); + return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), paramList.toArray(), this.clientMapper); } - private String buildSqlStringFromClientCriteria(final SearchParameters searchParameters) { + private String buildSqlStringFromClientCriteria(String schemaSql, final SearchParameters searchParameters, List<Object> paramList) { String sqlSearch = searchParameters.getSqlSearch(); final Long officeId = searchParameters.getOfficeId(); @@ -228,32 +231,38 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService sqlSearch = sqlSearch.replaceAll(" display_name ", " c.display_name "); sqlSearch = sqlSearch.replaceAll("display_name ", "c.display_name "); extraCriteria = " and (" + sqlSearch + ")"; + this.columnValidator.validateSqlInjection(schemaSql, sqlSearch); } if (officeId != null) { - extraCriteria += " and c.office_id = " + officeId; + extraCriteria += " and c.office_id = ? "; + paramList.add(officeId); } if (externalId != null) { - extraCriteria += " and c.external_id like " + ApiParameterHelper.sqlEncodeString(externalId); + paramList.add(ApiParameterHelper.sqlEncodeString(externalId)); + extraCriteria += " and c.external_id like ? " ; } if (displayName != null) { //extraCriteria += " and concat(ifnull(c.firstname, ''), if(c.firstname > '',' ', '') , ifnull(c.lastname, '')) like " - extraCriteria += " and c.display_name like " - + ApiParameterHelper.sqlEncodeString("%" + displayName + "%"); + paramList.add(ApiParameterHelper.sqlEncodeString(ApiParameterHelper.sqlEncodeString("%" + displayName + "%"))); + extraCriteria += " and c.display_name like ? "; } if (firstname != null) { - extraCriteria += " and c.firstname like " + ApiParameterHelper.sqlEncodeString(firstname); + paramList.add(ApiParameterHelper.sqlEncodeString(firstname)); + extraCriteria += " and c.firstname like ? " ; } if (lastname != null) { - extraCriteria += " and c.lastname like " + ApiParameterHelper.sqlEncodeString(lastname); + paramList.add(ApiParameterHelper.sqlEncodeString(lastname)); + extraCriteria += " and c.lastname like ? "; } if (searchParameters.isScopedByOfficeHierarchy()) { - extraCriteria += " and o.hierarchy like " + ApiParameterHelper.sqlEncodeString(searchParameters.getHierarchy() + "%"); + paramList.add(ApiParameterHelper.sqlEncodeString(searchParameters.getHierarchy() + "%")); + extraCriteria += " and o.hierarchy like ? "; } if(searchParameters.isOrphansOnly()){ @@ -263,7 +272,6 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService if (StringUtils.isNotBlank(extraCriteria)) { extraCriteria = extraCriteria.substring(4); } - return extraCriteria; } @@ -297,8 +305,8 @@ public class ClientReadPlatformServiceImpl implements ClientReadPlatformService if (StringUtils.isNotBlank(extraCriteria)) { sql += " and (" + extraCriteria + ")"; - } - + this.columnValidator.validateSqlInjection(sql, extraCriteria); + } return this.jdbcTemplate.query(sql, this.lookupMapper, new Object[] {}); } http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/CenterReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/CenterReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/CenterReadPlatformServiceImpl.java index 625d1d6..f400843 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/CenterReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/CenterReadPlatformServiceImpl.java @@ -47,6 +47,8 @@ import org.apache.fineract.infrastructure.core.service.PaginationHelper; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.core.service.SearchParameters; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.organisation.office.data.OfficeData; import org.apache.fineract.organisation.office.service.OfficeReadPlatformService; import org.apache.fineract.organisation.staff.data.StaffData; @@ -90,6 +92,7 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService private final ConfigurationDomainService configurationDomainService; private final CalendarReadPlatformService calendarReadPlatformService; private final DateTimeFormatter formatter = DateTimeFormat.forPattern("yyyy-MM-dd"); + private final ColumnValidator columnValidator; // data mappers private final CenterDataMapper centerMapper = new CenterDataMapper(); @@ -104,7 +107,7 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService final ClientReadPlatformService clientReadPlatformService, final OfficeReadPlatformService officeReadPlatformService, final StaffReadPlatformService staffReadPlatformService, final CodeValueReadPlatformService codeValueReadPlatformService, final PaginationParametersDataValidator paginationParametersDataValidator, final ConfigurationDomainService configurationDomainService, - final CalendarReadPlatformService calendarReadPlatformService) { + final CalendarReadPlatformService calendarReadPlatformService, final ColumnValidator columnValidator) { this.context = context; this.clientReadPlatformService = clientReadPlatformService; this.jdbcTemplate = new JdbcTemplate(dataSource); @@ -114,41 +117,48 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService this.paginationParametersDataValidator = paginationParametersDataValidator; this.configurationDomainService = configurationDomainService; this.calendarReadPlatformService = calendarReadPlatformService; + this.columnValidator = columnValidator; } // 'g.' preffix because of ERROR 1052 (23000): Column 'column_name' in where // clause is ambiguous // caused by the same name of columns in m_office and m_group tables - private String getCenterExtraCriteria(final SearchParameters searchCriteria) { + private String getCenterExtraCriteria(String schemaSl, List<Object> paramList,final SearchParameters searchCriteria) { StringBuffer extraCriteria = new StringBuffer(200); extraCriteria.append(" and g.level_id = " + GroupTypes.CENTER.getId()); String sqlQueryCriteria = searchCriteria.getSqlSearch(); if (StringUtils.isNotBlank(sqlQueryCriteria)) { + SQLInjectionValidator.validateSQLInput(sqlQueryCriteria); sqlQueryCriteria = sqlQueryCriteria.replaceAll(" display_name ", " g.display_name "); sqlQueryCriteria = sqlQueryCriteria.replaceAll("display_name ", "g.display_name "); extraCriteria.append(" and (").append(sqlQueryCriteria).append(") "); + this.columnValidator.validateSqlInjection(schemaSl, sqlQueryCriteria); } final Long officeId = searchCriteria.getOfficeId(); if (officeId != null) { - extraCriteria.append(" and g.office_id = ").append(officeId); + extraCriteria.append(" and g.office_id = ? "); + paramList.add(officeId); } final String externalId = searchCriteria.getExternalId(); if (externalId != null) { - extraCriteria.append(" and g.external_id = ").append(ApiParameterHelper.sqlEncodeString(externalId)); + paramList.add(ApiParameterHelper.sqlEncodeString(externalId)); + extraCriteria.append(" and g.external_id = ? "); } final String name = searchCriteria.getName(); if (name != null) { - extraCriteria.append(" and g.display_name like ").append(ApiParameterHelper.sqlEncodeString(name + "%")); + paramList.add(ApiParameterHelper.sqlEncodeString(name + "%")); + extraCriteria.append(" and g.display_name like ? "); } final String hierarchy = searchCriteria.getHierarchy(); if (hierarchy != null) { - extraCriteria.append(" and o.hierarchy like ").append(ApiParameterHelper.sqlEncodeString(hierarchy + "%")); + paramList.add(ApiParameterHelper.sqlEncodeString(hierarchy + "%")); + extraCriteria.append(" and o.hierarchy like ? "); } if (StringUtils.isNotBlank(extraCriteria.toString())) { @@ -157,7 +167,8 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService final Long staffId = searchCriteria.getStaffId(); if (staffId != null) { - extraCriteria.append(" and g.staff_id = ").append(staffId); + paramList.add(staffId); + extraCriteria.append(" and g.staff_id = ? "); } return extraCriteria.toString(); @@ -372,9 +383,10 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService sqlBuilder.append("select SQL_CALC_FOUND_ROWS "); sqlBuilder.append(this.centerMapper.schema()); sqlBuilder.append(" where o.hierarchy like ?"); - - final String extraCriteria = getCenterExtraCriteria(searchParameters); - + List<Object> paramList = new ArrayList<>( + Arrays.asList(hierarchySearchString)); + final String extraCriteria = getCenterExtraCriteria(this.centerMapper.schema(), paramList, searchParameters); + this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria); if (StringUtils.isNotBlank(extraCriteria)) { sqlBuilder.append(" and (").append(extraCriteria).append(")"); } @@ -392,7 +404,7 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService final String sqlCountRows = "SELECT FOUND_ROWS()"; return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), - new Object[] { hierarchySearchString }, this.centerMapper); + paramList.toArray(), this.centerMapper); } @Override @@ -407,9 +419,11 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService sqlBuilder.append("select "); sqlBuilder.append(this.centerMapper.schema()); sqlBuilder.append(" where o.hierarchy like ?"); - - final String extraCriteria = getCenterExtraCriteria(searchParameters); - + List<Object> paramList = new ArrayList<>( + Arrays.asList(hierarchySearchString)); + + final String extraCriteria = getCenterExtraCriteria(this.centerMapper.schema(), paramList, searchParameters); + this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria); if (StringUtils.isNotBlank(extraCriteria)) { sqlBuilder.append(" and (").append(extraCriteria).append(")"); } @@ -425,7 +439,7 @@ public class CenterReadPlatformServiceImpl implements CenterReadPlatformService } } - return this.jdbcTemplate.query(sqlBuilder.toString(), this.centerMapper, new Object[] { hierarchySearchString }); + return this.jdbcTemplate.query(sqlBuilder.toString(), this.centerMapper, paramList.toArray()); } @Override http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java index 608680a..8dd0ed6 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java @@ -39,6 +39,8 @@ import org.apache.fineract.infrastructure.core.service.PaginationHelper; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.core.service.SearchParameters; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.organisation.office.data.OfficeData; import org.apache.fineract.organisation.office.service.OfficeReadPlatformService; import org.apache.fineract.organisation.staff.data.StaffData; @@ -70,6 +72,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { private final AllGroupTypesDataMapper allGroupTypesDataMapper = new AllGroupTypesDataMapper(); private final PaginationHelper<GroupGeneralData> paginationHelper = new PaginationHelper<>(); private final PaginationParametersDataValidator paginationParametersDataValidator; + private final ColumnValidator columnValidator; private final static Set<String> supportedOrderByValues = new HashSet<>(Arrays.asList("id", "name", "officeId", "officeName")); @@ -78,7 +81,8 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { final CenterReadPlatformService centerReadPlatformService, final OfficeReadPlatformService officeReadPlatformService, final StaffReadPlatformService staffReadPlatformService, final CodeValueReadPlatformService codeValueReadPlatformService, - final PaginationParametersDataValidator paginationParametersDataValidator) { + final PaginationParametersDataValidator paginationParametersDataValidator, + final ColumnValidator columnValidator) { this.context = context; this.jdbcTemplate = new JdbcTemplate(dataSource); this.centerReadPlatformService = centerReadPlatformService; @@ -86,6 +90,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { this.staffReadPlatformService = staffReadPlatformService; this.codeValueReadPlatformService = codeValueReadPlatformService; this.paginationParametersDataValidator = paginationParametersDataValidator; + this.columnValidator = columnValidator; } @Override @@ -147,9 +152,10 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { sqlBuilder.append("select SQL_CALC_FOUND_ROWS "); sqlBuilder.append(this.allGroupTypesDataMapper.schema()); sqlBuilder.append(" where o.hierarchy like ?"); - - final String extraCriteria = getGroupExtraCriteria(searchParameters); - + List<Object> paramList = new ArrayList<>( + Arrays.asList(hierarchySearchString)); + final String extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), paramList, searchParameters); + this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria); if (StringUtils.isNotBlank(extraCriteria)) { sqlBuilder.append(" and (").append(extraCriteria).append(")"); } @@ -167,7 +173,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { final String sqlCountRows = "SELECT FOUND_ROWS()"; return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), - new Object[] { hierarchySearchString }, this.allGroupTypesDataMapper); + paramList.toArray(), this.allGroupTypesDataMapper); } @Override @@ -180,9 +186,9 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { sqlBuilder.append("select "); sqlBuilder.append(this.allGroupTypesDataMapper.schema()); sqlBuilder.append(" where o.hierarchy like ?"); - - final String extraCriteria = getGroupExtraCriteria(searchParameters); - + List<Object> paramList = new ArrayList<>( + Arrays.asList(hierarchySearchString)); + final String extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), paramList, searchParameters); if (StringUtils.isNotBlank(extraCriteria)) { sqlBuilder.append(" and (").append(extraCriteria).append(")"); } @@ -195,45 +201,52 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { sqlBuilder.append(parameters.limitSql()); } - return this.jdbcTemplate.query(sqlBuilder.toString(), this.allGroupTypesDataMapper, new Object[] { hierarchySearchString }); + return this.jdbcTemplate.query(sqlBuilder.toString(), this.allGroupTypesDataMapper, paramList.toArray()); } // 'g.' preffix because of ERROR 1052 (23000): Column 'column_name' in where // clause is ambiguous // caused by the same name of columns in m_office and m_group tables - private String getGroupExtraCriteria(final SearchParameters searchCriteria) { + private String getGroupExtraCriteria(String schemaSql, List<Object> paramList, final SearchParameters searchCriteria) { StringBuffer extraCriteria = new StringBuffer(200); extraCriteria.append(" and g.level_Id = ").append(GroupTypes.GROUP.getId()); String sqlSearch = searchCriteria.getSqlSearch(); if (sqlSearch != null) { + SQLInjectionValidator.validateSQLInput(sqlSearch); sqlSearch = sqlSearch.replaceAll(" display_name ", " g.display_name "); sqlSearch = sqlSearch.replaceAll("display_name ", "g.display_name "); extraCriteria.append(" and ( ").append(sqlSearch).append(") "); + this.columnValidator.validateSqlInjection(schemaSql, sqlSearch); } final Long officeId = searchCriteria.getOfficeId(); if (officeId != null) { - extraCriteria.append(" and g.office_id = ").append(officeId); + paramList.add(officeId); + extraCriteria.append(" and g.office_id = ? "); } final String externalId = searchCriteria.getExternalId(); if (externalId != null) { - extraCriteria.append(" and g.external_id = ").append(ApiParameterHelper.sqlEncodeString(externalId)); + paramList.add(ApiParameterHelper.sqlEncodeString(externalId)); + extraCriteria.append(" and g.external_id = ? "); } final String name = searchCriteria.getName(); if (name != null) { - extraCriteria.append(" and g.display_name like ").append(ApiParameterHelper.sqlEncodeString("%" + name + "%")); + paramList.add(ApiParameterHelper.sqlEncodeString("%" + name + "%")); + extraCriteria.append(" and g.display_name like ? "); } final String hierarchy = searchCriteria.getHierarchy(); if (hierarchy != null) { - extraCriteria.append(" and o.hierarchy like ").append(ApiParameterHelper.sqlEncodeString(hierarchy + "%")); + paramList.add(ApiParameterHelper.sqlEncodeString(hierarchy + "%")); + extraCriteria.append(" and o.hierarchy like ? "); } if (searchCriteria.isStaffIdPassed()) { - extraCriteria.append(" and g.staff_id = ").append(searchCriteria.getStaffId()); + paramList.add(searchCriteria.getStaffId()); + extraCriteria.append(" and g.staff_id = ? "); } if (StringUtils.isNotBlank(extraCriteria.toString())) { @@ -242,7 +255,8 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService { final Long staffId = searchCriteria.getStaffId(); if (staffId != null) { - extraCriteria.append(" and g.staff_id = ").append(staffId); + paramList.add(staffId); + extraCriteria.append(" and g.staff_id = ? "); } if(searchCriteria.isOrphansOnly()){ http://git-wip-us.apache.org/repos/asf/incubator-fineract/blob/b7a5e67b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java ---------------------------------------------------------------------- diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java index 4bf7326..3920ab2 100755 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java @@ -44,6 +44,8 @@ import org.apache.fineract.infrastructure.core.service.PaginationHelper; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; import org.apache.fineract.infrastructure.core.service.SearchParameters; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.apache.fineract.infrastructure.security.utils.ColumnValidator; +import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.organisation.monetary.data.CurrencyData; import org.apache.fineract.organisation.monetary.domain.ApplicationCurrency; import org.apache.fineract.organisation.monetary.domain.ApplicationCurrencyRepositoryWrapper; @@ -153,6 +155,7 @@ public class LoanReadPlatformServiceImpl implements LoanReadPlatformService { private final LoanUtilService loanUtilService; private final ConfigurationDomainService configurationDomainService; private final AccountDetailsReadPlatformService accountDetailsReadPlatformService; + private final ColumnValidator columnValidator; @Autowired public LoanReadPlatformServiceImpl(final PlatformSecurityContext context, final ApplicationCurrencyRepositoryWrapper applicationCurrencyRepository, @@ -166,7 +169,7 @@ public class LoanReadPlatformServiceImpl implements LoanReadPlatformService { final FloatingRatesReadPlatformService floatingRatesReadPlatformService, final LoanUtilService loanUtilService, final ConfigurationDomainService configurationDomainService, final AccountDetailsReadPlatformService accountDetailsReadPlatformService, - final LoanRepositoryWrapper loanRepositoryWrapper) { + final LoanRepositoryWrapper loanRepositoryWrapper, final ColumnValidator columnValidator) { this.context = context; this.loanRepositoryWrapper = loanRepositoryWrapper ; this.applicationCurrencyRepository = applicationCurrencyRepository; @@ -187,6 +190,7 @@ public class LoanReadPlatformServiceImpl implements LoanReadPlatformService { this.loanUtilService = loanUtilService; this.configurationDomainService = configurationDomainService; this.accountDetailsReadPlatformService = accountDetailsReadPlatformService; + this.columnValidator = columnValidator; } @Override @@ -281,7 +285,9 @@ public class LoanReadPlatformServiceImpl implements LoanReadPlatformService { String sqlQueryCriteria = searchParameters.getSqlSearch(); if (StringUtils.isNotBlank(sqlQueryCriteria)) { + SQLInjectionValidator.validateSQLInput(sqlQueryCriteria); sqlQueryCriteria = sqlQueryCriteria.replaceAll("accountNo", "l.account_no"); + this.columnValidator.validateSqlInjection(sqlBuilder.toString(), sqlQueryCriteria); sqlBuilder.append(" and (").append(sqlQueryCriteria).append(")"); }
