Fix some reporting issues including SQLi vulnerabilities (FINERACT-854)

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) {

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();
-        }
-    }
 }