diff --git a/frappe/__init__.py b/frappe/__init__.py index 2a1883c033..fa2540f706 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -14,7 +14,7 @@ import os, sys, importlib, inspect, json from .exceptions import * from .utils.jinja import get_jenv, get_template, render_template, get_email_from_template -__version__ = '10.0.22' +__version__ = '10.0.23' __title__ = "Frappe Framework" local = Local() diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index b748ae97c2..d2d8d8a621 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -3,6 +3,8 @@ from __future__ import unicode_literals +import six + import re, copy, os import frappe from frappe import _ @@ -381,7 +383,10 @@ class DocType(Document): # a DocType's name should not start with a number or underscore # and should only contain letters, numbers and underscore - is_a_valid_name = re.match("^(?![\W])[^\d_\s][\w -]+$", name, re.UNICODE) + if six.PY2: + is_a_valid_name = re.match("^(?![\W])[^\d_\s][\w -]+$", name) + else: + is_a_valid_name = re.match("^(?![\W])[^\d_\s][\w -]+$", name, flags = re.ASCII) if not is_a_valid_name: frappe.throw(_("DocType's name should start with a letter and it can only consist of letters, numbers, spaces and underscores"), frappe.NameError) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 25cd8e5e67..d11d7f0a51 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -159,10 +159,14 @@ class Report(Document): if as_dict: data = [] for row in out: - _row = frappe._dict() + if isinstance(row, (list, tuple)): + _row = frappe._dict() + for i, val in enumerate(row): + _row[columns[i].get('fieldname')] = val + elif isinstance(row, dict): + # no need to convert from dict to dict + _row = frappe._dict(row) data.append(_row) - for i, val in enumerate(row): - _row[columns[i].get('fieldname')] = val else: data = out return columns, data diff --git a/frappe/desk/moduleview.py b/frappe/desk/moduleview.py index d365f3d511..0b4f1eb853 100644 --- a/frappe/desk/moduleview.py +++ b/frappe/desk/moduleview.py @@ -124,10 +124,11 @@ def get_doctype_info(module): }, or_filters={ "ifnull(restrict_to_domain, '')": "", "restrict_to_domain": ("in", active_domains) - }, fields=["'doctype' as type", "name", "description", "ifnull(document_type, '') as document_type", + }, fields=["'doctype' as type", "name", "description", "document_type", "custom", "issingle"], order_by="custom asc, document_type desc, name asc") for d in doctype_info: + d.document_type = d.document_type or "" d.description = _(d.description or "") return doctype_info diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 54835250bf..5d455b2930 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -7,7 +7,7 @@ from six import iteritems, string_types """build query for doclistview and return results""" -import frappe, json, copy +import frappe, json, copy, re import frappe.defaults import frappe.share import frappe.permissions @@ -112,6 +112,7 @@ class DatabaseQuery(object): def prepare_args(self): self.parse_args() + self.sanitize_fields() self.extract_tables() self.set_optional_columns() self.build_conditions() @@ -177,6 +178,35 @@ class DatabaseQuery(object): filters.append(make_filter_tuple(self.doctype, key, value)) setattr(self, filter_name, filters) + def sanitize_fields(self): + ''' + regex : ^.*[,();].* + purpose : The regex will look for malicious patterns like `,`, '(', ')', ';' in each + field which may leads to sql injection. + example : + field = "`DocType`.`issingle`, version()" + + As field contains `,` and mysql function `version()`, with the help of regex + the system will filter out this field. + ''' + regex = re.compile('^.*[,();].*') + blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case'] + blacklisted_functions = ['concat', 'concat_ws', 'if', 'ifnull', 'nullif', 'coalesce', + 'connection_id', 'current_user', 'database', 'last_insert_id', 'session_user', + 'system_user', 'user', 'version'] + + def _raise_exception(): + frappe.throw(_('Cannot use sub-query or function in fields'), frappe.DataError) + + for field in self.fields: + if regex.match(field): + if any(keyword in field.lower() for keyword in blacklisted_keywords): + _raise_exception() + + if any("{0}(".format(keyword) in field.lower() \ + for keyword in blacklisted_functions): + _raise_exception() + def extract_tables(self): """extract tables from fields""" self.tables = ['`tab' + self.doctype + '`'] diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 359238be08..ad747e8c63 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -52,8 +52,7 @@ def rename_doc(doctype, old, new, force=False, merge=False, ignore_permissions=F update_attachments(doctype, old, new) - if merge: - frappe.delete_doc(doctype, old) + rename_versions(doctype, old, new) # call after_rename new_doc = frappe.get_doc(doctype, new) @@ -63,21 +62,23 @@ def rename_doc(doctype, old, new, force=False, merge=False, ignore_permissions=F new_doc.run_method("after_rename", old, new, merge) - rename_versions(doctype, old, new) - if not merge: rename_password(doctype, old, new) # update user_permissions frappe.db.sql("""update tabDefaultValue set defvalue=%s where parenttype='User Permission' and defkey=%s and defvalue=%s""", (new, doctype, old)) - frappe.clear_cache() if merge: new_doc.add_comment('Edit', _("merged {0} into {1}").format(frappe.bold(old), frappe.bold(new))) else: new_doc.add_comment('Edit', _("renamed from {0} to {1}").format(frappe.bold(old), frappe.bold(new))) + if merge: + frappe.delete_doc(doctype, old) + + frappe.clear_cache() + return new def update_attachments(doctype, old, new): diff --git a/frappe/public/js/frappe/form/controls/dynamic_link.js b/frappe/public/js/frappe/form/controls/dynamic_link.js index bcbcc32eb2..3da593edb8 100644 --- a/frappe/public/js/frappe/form/controls/dynamic_link.js +++ b/frappe/public/js/frappe/form/controls/dynamic_link.js @@ -1,11 +1,12 @@ frappe.ui.form.ControlDynamicLink = frappe.ui.form.ControlLink.extend({ get_options: function() { + let options = ''; if(this.df.get_options) { - return this.df.get_options(); + options = this.df.get_options(); } if (this.docname==null && cur_dialog) { //for dialog box - return cur_dialog.get_value(this.df.options); + options = cur_dialog.get_value(this.df.options); } if (!cur_frm) { const selector = `input[data-fieldname="${this.df.options}"]`; @@ -18,14 +19,15 @@ frappe.ui.form.ControlDynamicLink = frappe.ui.form.ControlLink.extend({ input = $(cur_page.page).find(selector); } if (input) { - return input.val(); + options = input.val(); } } - var options = frappe.model.get_value(this.df.parent, this.docname, this.df.options); - // if(!options) { - // frappe.msgprint(__("Please set {0} first", - // [frappe.meta.get_docfield(this.df.parent, this.df.options, this.docname).label])); - // } + options = frappe.model.get_value(this.df.parent, this.docname, this.df.options); + + if (frappe.model.is_single(options)) { + frappe.throw(__(`${options.bold()} is not a valid DocType for Dynamic Link`)); + } + return options; }, }); diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 5f50d558a1..e42e9df15c 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -509,9 +509,9 @@ frappe.ui.form.Grid = Class.extend({ } df.colsize = colsize; } - + // attach formatter on refresh - if (df.fieldtype == 'Link' && !df.formatter) { + if (df.fieldtype == 'Link' && !df.formatter && frappe.meta.docfield_map[df.parent]) { const docfield = frappe.meta.docfield_map[df.parent][df.fieldname]; if (docfield && docfield.formatter) { df.formatter = docfield.formatter; diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index bad75d3c77..484a309534 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -97,6 +97,42 @@ class TestReportview(unittest.TestCase): self.assertTrue(get_filters_cond('DocType', dict(istable=1), [], ignore_permissions=True)) frappe.set_user('Administrator') + def test_query_fields_sanitizer(self): + self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute, + fields=["name", "issingle, version()"], limit_start=0, limit_page_length=1) + + self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute, + fields=["name", "issingle, IF(issingle=1, (select name from tabUser), count(name))"], + limit_start=0, limit_page_length=1) + + self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute, + fields=["name", "issingle, (select count(*) from tabSessions)"], + limit_start=0, limit_page_length=1) + + self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute, + fields=["name", "issingle, SELECT LOCATE('', `tabUser`.`user`) AS user;"], + limit_start=0, limit_page_length=1) + + self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute, + fields=["name", "issingle, IF(issingle=1, (SELECT name from tabUser), count(*))"], + limit_start=0, limit_page_length=1) + + data = DatabaseQuery("DocType").execute(fields=["name", "issingle", "count(name)"], + limit_start=0, limit_page_length=1) + self.assertTrue('count(name)' in data[0]) + + data = DatabaseQuery("DocType").execute(fields=["name", "issingle", "locate('', name) as _relevance"], + limit_start=0, limit_page_length=1) + self.assertTrue('_relevance' in data[0]) + + data = DatabaseQuery("DocType").execute(fields=["name", "issingle", "date(creation) as creation"], + limit_start=0, limit_page_length=1) + self.assertTrue('creation' in data[0]) + + data = DatabaseQuery("DocType").execute(fields=["name", "issingle", + "datediff(modified, creation) as date_diff"], limit_start=0, limit_page_length=1) + self.assertTrue('date_diff' in data[0]) + def create_event(subject="_Test Event", starts_on=None): """ create a test event """