Skip to content

FINERACT-2461: Replace SQL string concatenation with parameterized queries#5492

Open
crambo70 wants to merge 1 commit intoapache:developfrom
crambo70:FINERACT-2461-parameterize-sql-queries
Open

FINERACT-2461: Replace SQL string concatenation with parameterized queries#5492
crambo70 wants to merge 1 commit intoapache:developfrom
crambo70:FINERACT-2461-parameterize-sql-queries

Conversation

@crambo70
Copy link

Summary

Replaces ~30 SQL injection vulnerabilities across 7 files with JDBC PreparedStatement parameterized queries. These vulnerabilities existed in the datatable, survey, email, and deposit account services — core infrastructure used by microfinance institutions serving unbanked populations worldwide.

Do good with AI.

Changes

  • DatatableWriteServiceImpl: Replace string-concatenated DELETE, INSERT, and SELECT queries with ? placeholders. Refactor permission registration from raw SQL string building (getPermissionSql()) to parameterized registerPermissions() method. Fixes injection vectors in deleteColumnCodeMapping, registerColumnCodeMapping, getCodeIdForColumn, parseDatatableColumnForDrop, isDatatableAttachedToEntityDatatableCheck, deregisterDatatable, and datatable entry deletion.
  • DatatableUtil: Parameterize all appTableId (Long) and officeHierarchy (String LIKE) values in dataScopedSQL() across all entity scoping branches (LOAN, SAVINGS, SAVINGS_TRANSACTION, CLIENT, GROUP, CENTER, OFFICE, PRODUCT). Refactor getOfficeJoinCondition, getGroupOfficeJoinCondition, and getClientOfficeJoinCondition helpers to use ? placeholders. Parameterize retrieveDataTableGenericResultSet().
  • GenericDataService/Impl: Add parameterized overload fillResultsetRowData(sql, headers, args...) to support prepared statement execution from DatatableUtil.
  • DatatableReadServiceImpl: Parameterize appTableId in countDatatableEntries() WHERE clause.
  • WriteSurveyServiceImpl: Remove duplicate vulnerable getPermissionSql() and unused dependencies (DatabaseSpecificSQLGenerator, DatatableReadService). Delegate to safe DatatableWriteService.registerDatatable(command).
  • DepositAccountReadPlatformServiceImpl: Add missing paginationParametersDataValidator.validateParameterValues() call in retrieveAll() to match the validation already present in retrieveAllPaged().

Test plan

  • ./gradlew compileJava — BUILD SUCCESSFUL (111 tasks)
  • ./gradlew test -x :twofactor-tests:test -x :oauth2-tests:test — all tests pass
  • Integration tests with MariaDB/PostgreSQL tenant databases (requires Docker environment)
  • Manual verification of datatable CRUD operations via REST API
  • Manual verification of survey registration
  • Manual verification of deposit account listing with pagination parameters

🤖 Generated with Claude Code

@IOhacker
Copy link
Contributor

Run './gradlew :fineract-provider:spotlessApply' to fix these violations.

…eries

Do good with AI.

~30 SQL injection vulnerabilities fixed across 7 files in a banking
platform that serves the unbanked. Every one of these was a potential
vector for someone to steal data from people who have the least to lose.

- DatatableWriteServiceImpl: Replace string-concatenated DELETE, INSERT,
  and SELECT queries with PreparedStatement parameters. Refactor
  permission registration from raw SQL string building to parameterized
  inserts. Remove getPermissionSql() in favor of registerPermissions().
- DatatableUtil: Parameterize all appTableId and officeHierarchy values
  in dataScopedSQL() across LOAN, SAVINGS, SAVINGS_TRANSACTION, CLIENT,
  GROUP, CENTER, OFFICE, and PRODUCT entity scoping queries. Refactor
  join condition helpers to use ? placeholders instead of string
  interpolation.
- GenericDataService/Impl: Add parameterized overload of
  fillResultsetRowData(sql, headers, args...) to support prepared
  statement execution.
- DatatableReadServiceImpl: Parameterize appTableId in
  countDatatableEntries WHERE clause.
- WriteSurveyServiceImpl: Remove duplicate vulnerable getPermissionSql()
  method and unused dependencies. Delegate to safe service method.
- DepositAccountReadPlatformServiceImpl: Add missing pagination parameter
  validation in retrieveAll() to match retrieveAllPaged() behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@crambo70 crambo70 force-pushed the FINERACT-2461-parameterize-sql-queries branch from ba97ad1 to 94422c0 Compare February 14, 2026 18:11
Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any possibility to add test classes for these changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants