Skip to content

fix: parameterise report SQL instead of formatting filters in - #77

Merged
aakvatech merged 18 commits into
Aakvatech-Limited:version-16-hotfixfrom
av-dev2:fix/report-sql-injection
Sep 4, 2026
Merged

fix: parameterise report SQL instead of formatting filters in#77
aakvatech merged 18 commits into
Aakvatech-Limited:version-16-hotfixfrom
av-dev2:fix/report-sql-injection

Conversation

@av-dev2

@av-dev2 av-dev2 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Removes the six frappe-sql-format-injection findings the Frappe semgrep rules
report against propms. Report filter values reached raw SQL through string
formatting, so any user who can open the report can inject SQL.

File Interpolated value
rent_invoices_details.py company, from_date, to_date, and the invoice id used for the per-invoice item query
rent_invoices_details_usd.py the same four
invoice_details/other_methods.py the rental filter and the invoice name, concatenated into the WHERE clause
utility_invoices/other_methods.py the invoice name, concatenated into the WHERE clause

Every one now travels as a %(name)s bind parameter. The only value still
concatenated into a query string is the conditions fragment in the two rent
reports, which is a constant literal chosen by an if, never user input.

The print(query) debug statement in the Utility Invoices rate lookup is gone
too.

semgrep --config frappe-semgrep-rules/rules on propms/ goes from 6
frappe-sql-format-injection findings to 0, and both reports still execute
against the version-16 test site.

Depends on #76.

av-dev2 and others added 2 commits September 3, 2026 15:36
# Conflicts:
#	.gitignore
#	propms/__init__.py
#	propms/auto_custom.py
#	propms/hooks.py
#	propms/issue_hook.py
#	propms/lease_invoice.py
#	propms/lease_invoice_schedule.py
#	propms/pos.js
#	propms/property_management_solution/doctype/lease/lease.py
#	propms/property_management_solution/doctype/property/property.py
#	propms/property_management_solution/report/invoice_details/other_methods.py
#	propms/property_management_solution/report/mis_income_break_up/other_methods.py
#	propms/property_management_solution/report/rent_invoices_details/rent_invoices_details.py
#	propms/property_management_solution/report/rent_invoices_details_usd/rent_invoices_details_usd.py
#	propms/property_management_solution/report/subscription_service_report/subscription_service_report.js
#	propms/property_management_solution/report/utility_invoices/other_methods.py
#	pyproject.toml
@aakvatech
aakvatech merged commit 438e82d into Aakvatech-Limited:version-16-hotfix Sep 4, 2026
3 of 4 checks passed
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