Skip to content

Commit

Permalink
fixes for CVE-2018-1289
Browse files Browse the repository at this point in the history
  • Loading branch information
vishwasbabu committed Feb 2, 2018
2 parents 17fd243 + e7035d1 commit 1d38bd2
Show file tree
Hide file tree
Showing 21 changed files with 146 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.apache.fineract.infrastructure.core.service.PaginationHelper;
import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.apache.fineract.organisation.monetary.data.CurrencyData;
import org.apache.fineract.organisation.office.data.OfficeData;
import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
Expand All @@ -74,18 +75,22 @@ public class JournalEntryReadPlatformServiceImpl implements JournalEntryReadPlat
private final JdbcTemplate jdbcTemplate;
private final GLAccountReadPlatformService glAccountReadPlatformService;
private final OfficeReadPlatformService officeReadPlatformService;
private final ColumnValidator columnValidator;
private final FinancialActivityAccountRepositoryWrapper financialActivityAccountRepositoryWrapper;

private final PaginationHelper<JournalEntryData> paginationHelper = new PaginationHelper<>();

@Autowired
public JournalEntryReadPlatformServiceImpl(final RoutingDataSource dataSource,
final GLAccountReadPlatformService glAccountReadPlatformService, final OfficeReadPlatformService officeReadPlatformService,
final GLAccountReadPlatformService glAccountReadPlatformService,
final ColumnValidator columnValidator,
final OfficeReadPlatformService officeReadPlatformService,
final FinancialActivityAccountRepositoryWrapper financialActivityAccountRepositoryWrapper) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.glAccountReadPlatformService = glAccountReadPlatformService;
this.officeReadPlatformService = officeReadPlatformService;
this.financialActivityAccountRepositoryWrapper = financialActivityAccountRepositoryWrapper;
this.columnValidator = columnValidator;
}

private static final class GLJournalEntryMapper implements RowMapper<JournalEntryData> {
Expand Down Expand Up @@ -356,9 +361,11 @@ public Page<JournalEntryData> retrieveAll(final SearchParameters searchParameter

if (searchParameters.isOrderByRequested()) {
sqlBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());

if (searchParameters.isSortOrderProvided()) {
sqlBuilder.append(' ').append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());
}
} else {
sqlBuilder.append(" order by journalEntry.entry_date, journalEntry.id");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,14 @@ public Page<AuditData> retrievePaginatedAuditEntries(final String extraCriteria,
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria);
if (parameters.isOrderByRequested()) {
sqlBuilder.append(' ').append(parameters.orderBySql());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), parameters.orderBySql());
} else {
sqlBuilder.append(' ').append(' ').append(" order by aud.id DESC");
}

if (parameters.isLimited()) {
sqlBuilder.append(' ').append(parameters.limitSql());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), parameters.limitSql());
}

logger.info("sql: " + sqlBuilder.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.fineract.infrastructure.jobs.data.JobDetailHistoryData;
import org.apache.fineract.infrastructure.jobs.exception.JobNotFoundException;
import org.apache.fineract.infrastructure.jobs.exception.OperationNotAllowedException;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
Expand All @@ -41,12 +42,15 @@
public class SchedulerJobRunnerReadServiceImpl implements SchedulerJobRunnerReadService {

private final JdbcTemplate jdbcTemplate;
private final ColumnValidator columnValidator;

private final PaginationHelper<JobDetailHistoryData> paginationHelper = new PaginationHelper<>();

@Autowired
public SchedulerJobRunnerReadServiceImpl(final RoutingDataSource dataSource) {
public SchedulerJobRunnerReadServiceImpl(final RoutingDataSource dataSource,
final ColumnValidator columnValidator) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.columnValidator = columnValidator;
}

@Override
Expand Down Expand Up @@ -79,9 +83,10 @@ public Page<JobDetailHistoryData> retrieveJobHistory(final Long jobId, final Sea
sqlBuilder.append(" where job.id=?");
if (searchParameters.isOrderByRequested()) {
sqlBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlBuilder.append(' ').append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getSortOrder());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.fineract.infrastructure.reportmailingjob.data.ReportMailingJobStretchyReportParamDateOption;
import org.apache.fineract.infrastructure.reportmailingjob.data.ReportMailingJobTimelineData;
import org.apache.fineract.infrastructure.reportmailingjob.exception.ReportMailingJobNotFoundException;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.joda.time.DateTime;
import org.joda.time.LocalDate;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -47,10 +48,13 @@
@Service
public class ReportMailingJobReadPlatformServiceImpl implements ReportMailingJobReadPlatformService {
private final JdbcTemplate jdbcTemplate;
private final ColumnValidator columnValidator;

@Autowired
public ReportMailingJobReadPlatformServiceImpl(final RoutingDataSource dataSource) {
public ReportMailingJobReadPlatformServiceImpl(final RoutingDataSource dataSource,
final ColumnValidator columnValidator) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.columnValidator = columnValidator;
}

@Override
Expand All @@ -66,9 +70,10 @@ public Page<ReportMailingJobData> retrieveAllReportMailingJobs(final SearchParam

if (searchParameters.isOrderByRequested()) {
sqlStringBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlStringBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlStringBuilder.append(" ").append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlStringBuilder.toString(), searchParameters.getSortOrder());
}
} else {
sqlStringBuilder.append(" order by rmj.name ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.reportmailingjob.data.ReportMailingJobRunHistoryData;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.joda.time.DateTime;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
Expand All @@ -39,12 +40,15 @@
public class ReportMailingJobRunHistoryReadPlatformServiceImpl implements ReportMailingJobRunHistoryReadPlatformService {
private final JdbcTemplate jdbcTemplate;
private final ReportMailingJobRunHistoryMapper reportMailingJobRunHistoryMapper;
private final ColumnValidator columnValidator;
private final PaginationHelper<ReportMailingJobRunHistoryData> paginationHelper = new PaginationHelper<>();

@Autowired
public ReportMailingJobRunHistoryReadPlatformServiceImpl(final RoutingDataSource dataSource) {
public ReportMailingJobRunHistoryReadPlatformServiceImpl(final RoutingDataSource dataSource,
final ColumnValidator columnValidator) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.reportMailingJobRunHistoryMapper = new ReportMailingJobRunHistoryMapper();
this.columnValidator = columnValidator;
}

@Override
Expand All @@ -63,9 +67,10 @@ public Page<ReportMailingJobRunHistoryData> retrieveRunHistoryByJobId(final Long

if (searchParameters.isOrderByRequested()) {
sqlStringBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlStringBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlStringBuilder.append(" ").append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlStringBuilder.toString(), searchParameters.getSortOrder());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,23 @@ private Set<String> getTableColumns(final ResultSet rs) {
return columns;
}

public void validateSqlInjection(String schema, String condition) {
SQLInjectionValidator.validateSQLInput(condition);
List<String> operator = new ArrayList<>(Arrays.asList("=", ">", "<",
"> =", "< =", "! =", "!=", ">=", "<="));
condition = condition.trim().replace("( ", "(").replace(" )", ")")
.toLowerCase();
for (String op : operator) {
condition = replaceAll(condition, op).replaceAll(" +", " ");
public void validateSqlInjection(String schema, String... conditions) {
for(String condition: conditions) {
SQLInjectionValidator.validateSQLInput(condition);
List<String> operator = new ArrayList<>(Arrays.asList("=", ">", "<",
"> =", "< =", "! =", "!=", ">=", "<="));
condition = condition.trim().replace("( ", "(").replace(" )", ")")
.toLowerCase();
for (String op : operator) {
condition = replaceAll(condition, op).replaceAll(" +", " ");
}
Set<String> operands = getOperand(condition);
schema = schema.trim().replaceAll(" +", " ").toLowerCase();
Map<String, Set<String>> tableColumnAliasMap = getTableColumnAliasMap(operands);
Map<String, Set<String>> tableColumnMap = getTableColumnMap(schema,
tableColumnAliasMap);
validateColumn(tableColumnMap);
}
Set<String> operands = getOperand(condition);
schema = schema.trim().replaceAll(" +", " ").toLowerCase();
Map<String, Set<String>> tableColumnAliasMap = getTableColumnAliasMap(operands);
Map<String, Set<String>> tableColumnMap = getTableColumnMap(schema,
tableColumnAliasMap);
validateColumn(tableColumnMap);
}

private static Map<String, Set<String>> getTableColumnMap(String schema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

public class SQLInjectionValidator {

private final static String[] DDL_COMMANDS = { "create", "drop", "alter", "truncate", "comment" };
private final static String[] DDL_COMMANDS = { "create", "drop", "alter", "truncate", "comment", "sleep" };

private final static String[] DML_COMMANDS = { "select", "insert", "update", "delete", "merge", "upsert", "call" };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.fineract.infrastructure.core.service.PaginationHelper;
import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.apache.fineract.infrastructure.sms.data.SmsData;
import org.apache.fineract.infrastructure.sms.domain.SmsMessageEnumerations;
import org.apache.fineract.infrastructure.sms.domain.SmsMessageStatusType;
Expand All @@ -49,11 +50,14 @@ public class SmsReadPlatformServiceImpl implements SmsReadPlatformService {
private final JdbcTemplate jdbcTemplate;
private final SmsMapper smsRowMapper;
private final PaginationHelper<SmsData> paginationHelper = new PaginationHelper<>();
private final ColumnValidator columnValidator;

@Autowired
public SmsReadPlatformServiceImpl(final RoutingDataSource dataSource) {
public SmsReadPlatformServiceImpl(final RoutingDataSource dataSource,
final ColumnValidator columnValidator) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.smsRowMapper = new SmsMapper();
this.columnValidator = columnValidator;
}

private static final class SmsMapper implements RowMapper<SmsData> {
Expand Down Expand Up @@ -224,9 +228,10 @@ public Page<SmsData> retrieveSmsByStatus(final Long campaignId, final SearchPara

if (searchParameters.isOrderByRequested()) {
sqlBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlBuilder.append(' ').append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getSortOrder());
}
} else {
sqlBuilder.append(" order by smo.submittedon_date, smo.id");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,18 @@
*/
package org.apache.fineract.notification.service;

import org.apache.fineract.infrastructure.core.service.*;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.List;

import org.apache.fineract.infrastructure.core.service.Page;
import org.apache.fineract.infrastructure.core.service.PaginationHelper;
import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.apache.fineract.notification.cache.CacheNotificationResponseHeader;
import org.apache.fineract.notification.data.NotificationData;
import org.apache.fineract.notification.data.NotificationMapperData;
Expand All @@ -28,26 +38,25 @@
import org.springframework.jdbc.core.RowMapper;
import org.springframework.stereotype.Service;

import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.List;

@Service
public class NotificationReadPlatformServiceImpl implements NotificationReadPlatformService {

private final JdbcTemplate jdbcTemplate;
private final PlatformSecurityContext context;
private final ColumnValidator columnValidator;
private final PaginationHelper<NotificationData> paginationHelper = new PaginationHelper<>();
private final NotificationDataRow notificationDataRow = new NotificationDataRow();
private final NotificationMapperRow notificationMapperRow = new NotificationMapperRow();
private HashMap<Long, HashMap<Long, CacheNotificationResponseHeader>>
tenantNotificationResponseHeaderCache = new HashMap<>();

@Autowired
public NotificationReadPlatformServiceImpl(final RoutingDataSource dataSource, final PlatformSecurityContext context) {
public NotificationReadPlatformServiceImpl(final RoutingDataSource dataSource,
final PlatformSecurityContext context,
final ColumnValidator columnValidator) {
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.context = context;
this.columnValidator = columnValidator;
}

@Override
Expand Down Expand Up @@ -139,9 +148,10 @@ private Page<NotificationData> getNotificationDataPage(SearchParameters searchPa

if (searchParameters.isOrderByRequested()) {
sqlBuilder.append(" order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlBuilder.append(' ').append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getSortOrder());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
import org.apache.fineract.infrastructure.core.service.SearchParameters;
import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
import org.apache.fineract.organisation.monetary.data.CurrencyData;
import org.apache.fineract.organisation.monetary.service.CurrencyReadPlatformService;
import org.apache.fineract.organisation.office.data.OfficeData;
Expand All @@ -48,13 +49,17 @@ public class OfficeReadPlatformServiceImpl implements OfficeReadPlatformService
private final JdbcTemplate jdbcTemplate;
private final PlatformSecurityContext context;
private final CurrencyReadPlatformService currencyReadPlatformService;
private final ColumnValidator columnValidator;
private final static String nameDecoratedBaseOnHierarchy = "concat(substring('........................................', 1, ((LENGTH(o.hierarchy) - LENGTH(REPLACE(o.hierarchy, '.', '')) - 1) * 4)), o.name)";

@Autowired
public OfficeReadPlatformServiceImpl(final PlatformSecurityContext context,
final CurrencyReadPlatformService currencyReadPlatformService, final RoutingDataSource dataSource) {
final CurrencyReadPlatformService currencyReadPlatformService,
final RoutingDataSource dataSource,
final ColumnValidator columnValidator) {
this.context = context;
this.currencyReadPlatformService = currencyReadPlatformService;
this.columnValidator = columnValidator;
this.jdbcTemplate = new JdbcTemplate(dataSource);
}

Expand Down Expand Up @@ -159,9 +164,10 @@ public Collection<OfficeData> retrieveAllOffices(final boolean includeAllOffices
if(searchParameters!=null) {
if (searchParameters.isOrderByRequested()) {
sqlBuilder.append("order by ").append(searchParameters.getOrderBy());

this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy());
if (searchParameters.isSortOrderProvided()) {
sqlBuilder.append(' ').append(searchParameters.getSortOrder());
this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getSortOrder());
}
} else {
sqlBuilder.append("order by o.hierarchy");
Expand Down
Loading

0 comments on commit 1d38bd2

Please sign in to comment.