FINERACT-2081: ESAPI.encoder().encodeForSQL is deprecated, so replaced the method with parameterized queries based solution with user input validation
This commit is contained in:
+31
@@ -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");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+56
-11
@@ -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");
|
||||
}
|
||||
|
||||
final String inputSql = "select " + encodedType + "_sql as the_sql from stretchy_" + encodedType + " where " + encodedType
|
||||
+ "_name = ?";
|
||||
// Validate report type against whitelist - this prevents SQL injection in table names
|
||||
if (!ReportType.isValidType(type)) {
|
||||
throw new IllegalArgumentException("Invalid report type provided");
|
||||
}
|
||||
|
||||
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);
|
||||
|
||||
+101
-7
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user