SQL queries in JS

1

Hi,

While going through the application files, I noticed that there are SQL queries being constructed in JS files (for example in account.js for the account doctype).
This design looks like it could have some security vulnerabilities, for example a user could alter the query on the client side to gain access to data that he is not authorized for.

It would be great if someone could shed some light on whether some counter-measures are being applied on the server side to prevent unauthorized access and any other form of SQL-based attack.

Thanks,
Aditya

–

You received this message because you are subscribed to the Google Groups "ERPNext Developer Forum" group.

To unsubscribe from this group and stop receiving emails from it, send an email to erpnext-developer-forum+un...@googlegroups.com.

To post to this group, send email to er...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/erpnext-developer-forum/-/oyusa-rEo4YJ.

For more options, visit https://groups.google.com/groups/opt_out.

 

 

2
Aditya,

Most (not all) of these queries are parsed / verified on the server side and appropriate permission rules are applied.

You are right this is a potential issue, whereby an authenticated user could get access to data that the user is not permitted to access. We plan to move these to the server side eventually.

Thanks for reminding this again. We were keeping this as a "good to have", but I think the priority for this should be much higher.

best,
Rushabh







On Tue, Feb 5, 2013 at 3:03 PM, Aditya Mukhopadhyay <Ad...@gocoop.com> wrote:
Hi,
While going through the application files, I noticed that there are SQL queries being constructed in JS files (for example in account.js for the account doctype).
This design looks like it could have some security vulnerabilities, for example a user could alter the query on the client side to gain access to data that he is not authorized for.

It would be great if someone could shed some light on whether some counter-measures are being applied on the server side to prevent unauthorized access and any other form of SQL-based attack.

Thanks,
Aditya

–

You received this message because you are subscribed to the Google Groups "ERPNext Developer Forum" group.

To unsubscribe from this group and stop receiving emails from it, send an email to erpnext-developer-forum+un…@googlegroups.com.

To post to this group, send email to er…@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/erpnext-developer-forum/-/oyusa-rEo4YJ.

For more options, visit https://groups.google.com/groups/opt_out.








–



–

You received this message because you are subscribed to the Google Groups "ERPNext Developer Forum" group.

To unsubscribe from this group and stop receiving emails from it, send an email to erpnext-developer-forum+un...@googlegroups.com.

To post to this group, send email to er...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

 

 

3

Hi,

I had seen these sql queries and assumed they must be getting verified server side - I’m very alarmed to see they’re not. Or at least they weren’t in Feb.

Could you kindly comment on whether this has changed since then?

Many thanks.

4

Thanks for checking up. SQL queries on JS have been removed.

5

Thanks. Do you mean in 5?

Because in 4.11 I still see the following:

sales_partner.js:

		get_query: function() {
			return "select name, address_type, address_line1, address_line2, city, state, country, pincode, fax, email_id, phone, is_primary_address, is_shipping_address from tabAddress where sales_partner='" +
				cur_frm.doc.name.replace(/'/g, "\\'") + "' and docstatus != 2 order by is_primary_address desc"
		},

customer.js

		get_query: function() {
			return "select name, address_type, address_line1, address_line2, city, state, country, pincode, fax, email_id, phone, is_primary_address, is_shipping_address from tabAddress where customer='" +
				cur_frm.doc.name.replace(/'/g, "\\'") + "' and docstatus != 2 order by is_primary_address desc"
		},

supplier.js:

		get_query: function() {
			return "select name, address_type, address_line1, address_line2, city, state, country, pincode, fax, email_id, phone, is_primary_address, is_shipping_address from tabAddress where supplier='" +
				cur_frm.doc.name.replace(/'/g, "\\'") + "' and docstatus != 2 order by is_primary_address desc"
		},
6

@thenon Thanks again. I thought we had fixed those.

Fixed via:

https://github.com/frappe/frappe/pull/923

and

https://github.com/frappe/erpnext/pull/2427

7

Great, so those controls are no longer being rendered that way, but for my own understanding please, what is it in either of those checkins that inhibits sql originating from the client continuing to being run by the server, through the same mechanism? (e.g. initiated through js console).

Thanks.

8

This line should block any such arbitrary query from being executed.