Addressing Improper Error Handling in frappe.desk.reportview.get_list

Frappe Framework
,
1

Hi Frappe Community,

During a recent security review, an Improper Error Handling concern was raised regarding the frappe.desk.reportview.get_list method. Here are the details and recommendations:

Current Implementation:

  • The get_list function is part of the Frappe framework’s core repository and is maintained as an ordinary whitelisted function.
  • The function uses the read_only flag to ensure it performs only read operations on the database, reducing potential risks.
  • Converting it to a dedicated API function may lead to unintended behavior in other parts of the framework, so no changes have been made to its structure.

Feedback:

  1. Penetration Tester Recommendation:
  • Mask or prevent the visibility of sensitive parameters such as exe and _server_messages on the client side.
  • Disable stack traces or debug mode for users without sufficient privileges to reduce exposure of sensitive information.
  1. SEM Feedback (27-11-24):
  • Risk level has been downgraded to Low.
  1. Livelawbazar Feedback (30-11-24):
  • Recommendation to create a ticket for the Frappe core team to address this issue further.

Proposed Next Steps:

  • Enhance Error Handling: Mask sensitive parameter values (exe, _server_messages, etc.) in client-side error responses.
  • Restrict Debug Information: Ensure stack traces or debug information are disabled for users without developer-level permissions.
  • Create a Ticket: Collaborate with the Frappe team to assess and implement these improvements without disrupting the current functionality.

Would these recommendations align with the framework’s current goals? Should we proceed with creating a ticket for this enhancement? Your insights and suggestions are highly appreciated!

Best regards,
Nakul P Kumar
Faircode Infotech Pvt. Ltd.