Permissions bugs: easy access to child tables for any logged in user

[Andrew_McLeod]

12 days ago I reported, via the reporting process for security issues in Frappe, a pair of permissions issues in Frappe that allow unrestricted access to child table information to a logged-in user. Both can be accessed through simple JS e.g. typing stuff in the JS console in Chrome.

Both attacks require the user to be logged in (I think).
The first attack does not require the user to be a member of any role.
The second attack also requires that the user has permissions for an arbitrary Doctype which does not need to be a parent of the child doctype being retrieved. Since even a Website User has access to the Communication doctype, this is again accessible to any logged in user.

I have successfully tested this on my own systems and (for validation only) it also works on support.erpnext.com. I haven’t tried it on the main erpnext site as I don’t have a login (although I think I would just have to sign up) and I couldn’t be bothered to make an account on demo.erpnext.com with restricted permissions.

I have not heard anything back from Frappe and will (when I get round to it) post the issues in more detail. Essentially as far as I can tell there is no child table permissions model - it’s not just broken; it doesn’t exist. The correct way to do this is probably to always inherit permission - since every child table entry has a parent, it can inherit their permission, but it can’t be done on a Doctype level because any given child table Doctype isn’t tied to a particular parent Doctype (the same child Doctype can be used in multiple parent Doctypes).

[Andrew_McLeod]

https://erpnext.com/report

is what I have already done (12 days ago), before anyone posts it. I have had no response.

Child Tables are often the ‘juicy’ bits of DocTypes, and many sites allow creation of Website User accounts, so this should probably have been a slightly higher priority?

[Andrew_McLeod]

I’m a little surprised by the lack of interest in this, either from the Frappe team or just generally.

Any logged in user can access any child table document (e.g. Sales/Purchase Receipt/Invoice/Order Item, Bank Statement Transaction Invoice/Payment/Settings Item, Salary Detail) without permission.

[littlehera]

The first attack does not require the user to be a member of any role.
The second attack also requires that the user has permissions for an arbitrary Doctype which does not need to be a parent of the child doctype being retrieved.

Would it be possible to know the step-by-step process on how you managed to discover/uncover the said security holes?

[Andrew_McLeod]

Finding the second attack was a result of playing around with this, and looking at the client-side JS code.

Issue: Client-side permission errors when adding Item Barcode to Item page

https://github.com/frappe/erpnext/issues/15886 As described on the Github issue, when typing the name of a Barcode on the Item Barcode page you get client permission errors. These aren’t actually too much of a problem as it’s just a bit of Javascript trying to query the database to see if any Item Barcodes with the name you have typed exist and if so provide an error message. The change is probably due to the restriction of access to child tables to the client. I also believe this is a Frappe,…

Finding the first attack was somewhat accidental while trying things in the JS console that I actually expected to fail.

The first is a whitelisted function which allows querying the DB. This does check for permissions but there aren’t any on child tables, so…
The second is because check_parent_permission is trivially (almost embarrassingly) avoidable - it would be OK if intended only for use by trusted code, but not for stuff from outside.

It’s been nearly 2 weeks since I sent emails off with no response so I assume there is no interest and I will post them here so people can discuss/develop patches.

[Andrew_McLeod]

The whitelisted method frappe.model.db_query does not check if the doctype provided is a child table, and consequently (as child tables have no permissions) allows unfettered access to child table data.
For example, run this in the JS console as an unprivileged, but logged in user to retrieve the item_code, item_name, amount and description of the first 20 Purchase Receipt Item entries…

frappe.call({method: 'frappe.model.db_query.get_list', args: {doctype: 'Purchase Receipt Item', limit: 20, fields: ['item_code', 'item_name', 'amount', 'description']}, callback: function(r) {console.log(r.message);}});

The second method relies on calling frappe.client.get_list. Now nominally this requires you to pass a parent Doctype to see if you have permission before accessing child tables. However, since this check (in check_parent_permission) is so embarrassingly insecure, you can simply pass any Doctype you do have access to - for example, Communication which I think any logged in user will have access to even if they don’t have access to the Desk.

This JS command will retrieve (again I think the first 20 due to an implicit limit parameter which can be overridden) Purchase Receipt Item entries.

frappe.call({method: 'frappe.client.get_list', args: {doctype: 'Purchase Receipt Item', fields: ['item_code', 'item_name', 'amount', 'description'], parent: 'Communication'}, callback: function(r) {console.log(r);}})

[Tommaso_Meacci1]

Hi Andrew,

I am struggling with this issue you raised some years ago. I am not very technical but I would like help to implement this extra level of security.
please let me know if you can help.

thank you