From 0d9b3680b716f15503ebadd6b60247759cd91556 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 8 Feb 2018 16:57:27 +0530 Subject: [PATCH 1/9] [hotfix] Disallow Single DocType for Dynamic Link (#4986) --- .../js/frappe/form/controls/dynamic_link.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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; }, }); From 8f0e0fc647a1b0f825e90fd9c3c86b0fa5c9210c Mon Sep 17 00:00:00 2001 From: Achilles Rasquinha Date: Fri, 9 Feb 2018 11:03:22 +0530 Subject: [PATCH 2/9] Fixed Unicode to ASCII doc --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 5c1aa65ca4..5dc31376fc 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -381,7 +381,7 @@ 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) + is_a_valid_name = re.match("^(?![\W])[^\d_\s][\w -]+$", name, 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) From 9790a8067a87c738ca91682307e3f05418396858 Mon Sep 17 00:00:00 2001 From: Achilles Rasquinha Date: Fri, 9 Feb 2018 11:22:18 +0530 Subject: [PATCH 3/9] PY2-PY3 compat --- frappe/core/doctype/doctype/doctype.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 5dc31376fc..4a9c7f3135 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -381,7 +381,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.ASCII) + 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) @@ -620,6 +623,7 @@ def validate_fields(meta): for d in fields: if not d.permlevel: d.permlevel = 0 if d.fieldtype != "Table": d.allow_bulk_edit = 0 + if d.fieldtype == "Barcode": d.ignore_xss_filter = 1 if not d.fieldname: frappe.throw(_("Fieldname is required in row {0}").format(d.idx)) d.fieldname = d.fieldname.lower() From 9743e5091ce23900a67621fbbd5ba85bd29d6910 Mon Sep 17 00:00:00 2001 From: Achilles Rasquinha Date: Fri, 9 Feb 2018 11:25:09 +0530 Subject: [PATCH 4/9] fixed import --- frappe/core/doctype/doctype/doctype.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 4a9c7f3135..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 _ From 99b4278f846149fbb4cff93c5c41e742fd64b63e Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 9 Feb 2018 14:28:07 +0530 Subject: [PATCH 5/9] [report] Handle rows which can be list of dict (#4987) --- frappe/core/doctype/report/report.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 From 8f2935a0e33c79bceadcc6aa67cc2f7d5f8fe901 Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Fri, 9 Feb 2018 17:34:55 +0530 Subject: [PATCH 6/9] [Fix] Formatter issue (#4993) --- frappe/public/js/frappe/form/grid.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 9b67442d9a..a5a14ac26c 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -508,9 +508,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; From 40d818af90a5771acda3796ef9b0439c7aa7d057 Mon Sep 17 00:00:00 2001 From: Saurabh Date: Tue, 13 Feb 2018 14:45:51 +0530 Subject: [PATCH 7/9] [hotfix] sanitize fields to avoid mysql injection (#4994) * [fix] sanitize fields to avoid mysql injection * sanitize sql statements to avoid subqueries * Added test cases * Raise exception if mysql injection found in fields and related test-cases * [fix] riase exception if blacklistes function or keyworkds found in fields --- frappe/desk/moduleview.py | 3 ++- frappe/model/db_query.py | 32 ++++++++++++++++++++++++++++++- frappe/tests/test_db_query.py | 36 +++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) 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 5349823a2c..dd0b386570 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 @@ -113,6 +113,7 @@ class DatabaseQuery(object): def prepare_args(self): self.parse_args() + self.sanitize_fields() self.extract_tables() self.set_optional_columns() self.build_conditions() @@ -178,6 +179,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/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 """ From 3e15c148bd5bcb4cf451a33286eaedd25df754ad Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 13 Feb 2018 16:01:58 +0530 Subject: [PATCH 8/9] [hotfix] Deadlock fix rename doc (#5001) * [deadlock fix] rename_version before delete_doc * Move delete_doc to end of rename_doc --- frappe/model/rename_doc.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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): From 396366c1e290d80c755a58fabb440eae7e86e23a Mon Sep 17 00:00:00 2001 From: Saurabh Date: Wed, 14 Feb 2018 12:12:59 +0600 Subject: [PATCH 9/9] bumped to version 10.0.23 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index ce9cf8f052..456e3ddab2 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()