You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by pt...@apache.org on 2021/04/12 22:11:11 UTC
[fineract] 09/11: Fix some reporting issues including SQLi
vulnerabilities (FINERACT-854)
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 <jo...@strathmore.edu>
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();
- }
- }
}