This is an automated email from the ASF dual-hosted git repository. ptuomola pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/fineract.git
commit b3d00252f5e90c340faa3ddb7e9b2eb954c8dad6 Author: Joseph Makara <[email protected]> AuthorDate: Sun Apr 4 15:28:24 2021 +0300 Fix some reporting issues including SQLi vulnerabilities (FINERACT-854) --- .../dataqueries/api/RunreportsApiResource.java | 10 ++++++- .../service/ReadReportingServiceImpl.java | 32 ++++++---------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java index 0f0c4c5..aa9bbb8 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java @@ -39,6 +39,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import org.apache.fineract.infrastructure.core.api.ApiParameterHelper; import org.apache.fineract.infrastructure.core.exception.PlatformServiceUnavailableException; +import org.apache.fineract.infrastructure.dataqueries.service.DatatableReportingProcessService; import org.apache.fineract.infrastructure.dataqueries.service.ReadReportingService; import org.apache.fineract.infrastructure.report.provider.ReportingProcessServiceProvider; import org.apache.fineract.infrastructure.report.service.ReportingProcessService; @@ -60,13 +61,16 @@ public class RunreportsApiResource { private final PlatformSecurityContext context; private final ReadReportingService readExtraDataAndReportingService; private final ReportingProcessServiceProvider reportingProcessServiceProvider; + private final DatatableReportingProcessService datatableReportingProcessService; @Autowired public RunreportsApiResource(final PlatformSecurityContext context, final ReadReportingService readExtraDataAndReportingService, - final ReportingProcessServiceProvider reportingProcessServiceProvider) { + final ReportingProcessServiceProvider reportingProcessServiceProvider, + DatatableReportingProcessService aDatatableReportingProcessService) { this.context = context; this.readExtraDataAndReportingService = readExtraDataAndReportingService; this.reportingProcessServiceProvider = reportingProcessServiceProvider; + datatableReportingProcessService = aDatatableReportingProcessService; } @GET @@ -105,6 +109,10 @@ public class RunreportsApiResource { // Pass through isSelfServiceUserReport so that ReportingProcessService implementations can use it queryParams.putSingle(IS_SELF_SERVICE_USER_REPORT_PARAMETER, Boolean.toString(isSelfServiceUserReport)); + if (parameterType) { + return datatableReportingProcessService.processRequest(reportName, queryParams); + } + String reportType = this.readExtraDataAndReportingService.getReportType(reportName, isSelfServiceUserReport); ReportingProcessService reportingProcessService = this.reportingProcessServiceProvider.findReportingProcessService(reportType); if (reportingProcessService == null) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java index 43584f3..cd538e3 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java @@ -38,7 +38,6 @@ import java.util.Map; import java.util.Set; import javax.sql.DataSource; import javax.ws.rs.core.StreamingOutput; -import org.apache.commons.lang3.StringUtils; import org.apache.fineract.infrastructure.core.domain.JdbcSupport; import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException; import org.apache.fineract.infrastructure.core.service.RoutingDataSource; @@ -51,8 +50,6 @@ import org.apache.fineract.infrastructure.dataqueries.data.ResultsetRowData; import org.apache.fineract.infrastructure.dataqueries.exception.ReportNotFoundException; import org.apache.fineract.infrastructure.documentmanagement.contentrepository.FileSystemContentRepository; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; -import org.apache.fineract.infrastructure.security.utils.ColumnValidator; -import org.apache.fineract.infrastructure.security.utils.SQLInjectionException; import org.apache.fineract.useradministration.domain.AppUser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,22 +63,19 @@ import org.springframework.stereotype.Service; public class ReadReportingServiceImpl implements ReadReportingService { private static final Logger LOG = LoggerFactory.getLogger(ReadReportingServiceImpl.class); - private static final String REPORT_NAME_REGEX_PATTERN = "^[a-zA-Z][a-zA-Z0-9\\-_\\s]{0,48}[a-zA-Z0-9\\s](\\([a-zA-Z]*\\))?$"; private final JdbcTemplate jdbcTemplate; private final DataSource dataSource; private final PlatformSecurityContext context; private final GenericDataService genericDataService; - private final ColumnValidator columnValidator; @Autowired public ReadReportingServiceImpl(final PlatformSecurityContext context, final RoutingDataSource dataSource, - final GenericDataService genericDataService, final ColumnValidator columnValidator) { + final GenericDataService genericDataService) { this.context = context; this.dataSource = dataSource; this.jdbcTemplate = new JdbcTemplate(this.dataSource); this.genericDataService = genericDataService; - this.columnValidator = columnValidator; } @Override @@ -204,13 +198,12 @@ public class ReadReportingServiceImpl implements ReadReportingService { } private String getSql(final String name, final String type) { - final String inputSql = "select " + type + "_sql as the_sql from stretchy_" + type + " where " + type + "_name = '" + name + "'"; - validateReportName(name); + final String inputSql = "select " + type + "_sql as the_sql from stretchy_" + type + " where " + type + "_name = ?"; final String inputSqlWrapped = this.genericDataService.wrapSQL(inputSql); // the return statement contains the exact sql required - final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(inputSqlWrapped); + final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(inputSqlWrapped, name); if (rs.next() && rs.getString("the_sql") != null) { return rs.getString("the_sql"); @@ -220,14 +213,11 @@ public class ReadReportingServiceImpl implements ReadReportingService { @Override public String getReportType(final String reportName, final boolean isSelfServiceUserReport) { - final String sql = "SELECT ifnull(report_type,'') as report_type FROM `stretchy_report` where report_name = '" + reportName - + "' and self_service_user_report = ?"; - validateReportName(reportName); - this.columnValidator.validateSqlInjection(sql, reportName); + final String sql = "SELECT ifNull(report_type,'') AS report_type FROM `stretchy_report` WHERE report_name = ? AND self_service_user_report = ?"; final String sqlWrapped = this.genericDataService.wrapSQL(sql); - final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(sqlWrapped, isSelfServiceUserReport); + final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(sqlWrapped, reportName, isSelfServiceUserReport); if (rs.next()) { return rs.getString("report_type"); @@ -323,7 +313,8 @@ public class ReadReportingServiceImpl implements ReadReportingService { final String sql = rm.schema(id); - final Collection<ReportParameterJoinData> rpJoins = this.jdbcTemplate.query(sql, rm); + final Collection<ReportParameterJoinData> rpJoins = this.jdbcTemplate.query(sql, rm, + id != null ? new Object[] { id } : new Object[] {}); final Collection<ReportData> reportList = new ArrayList<>(); if (rpJoins == null || rpJoins.size() == 0) { @@ -416,7 +407,7 @@ public class ReadReportingServiceImpl implements ReadReportingService { sql += " from stretchy_report r" + " left join stretchy_report_parameter rp on rp.report_id = r.id" + " left join stretchy_parameter p on p.id = rp.parameter_id"; if (reportId != null) { - sql += " where r.id = " + reportId; + sql += " where r.id = ?"; } else { sql += " order by r.id, rp.parameter_id"; } @@ -498,7 +489,6 @@ public class ReadReportingServiceImpl implements ReadReportingService { final Set<String> keys = queryParams.keySet(); for (String key : keys) { final String pValue = queryParams.get(key); - // LOG.info("(" + key + " : " + pValue + ")"); key = "${" + key + "}"; sql = this.genericDataService.replace(sql, key, pValue); } @@ -568,10 +558,4 @@ public class ReadReportingServiceImpl implements ReadReportingService { */ return null; } - - private void validateReportName(final String name) { - if (!StringUtils.isBlank(name) && !name.matches(REPORT_NAME_REGEX_PATTERN)) { - throw new SQLInjectionException(); - } - } }
