From 472d33f3da5e9410d9fc4d2e50e154d0794a4885 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Mon, 29 Jul 2019 17:07:09 +0530 Subject: [PATCH 01/14] fix(security): Make Jinja Tighter --- frappe/utils/jinja.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/utils/jinja.py b/frappe/utils/jinja.py index 7a27fb3c3b..28e3b3d463 100644 --- a/frappe/utils/jinja.py +++ b/frappe/utils/jinja.py @@ -6,10 +6,11 @@ def get_jenv(): import frappe if not getattr(frappe.local, 'jenv', None): - from jinja2 import Environment, DebugUndefined + from jinja2 import DebugUndefined + from jinja2.sandbox import SandboxedEnvironment # frappe will be loaded last, so app templates will get precedence - jenv = Environment(loader = get_jloader(), + jenv = SandboxedEnvironment(loader = get_jloader(), undefined=DebugUndefined) set_filters(jenv) From 8ac155f7b6d180f8ba7da6d5678e51cb2316f9a7 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Mon, 29 Jul 2019 20:22:04 +0530 Subject: [PATCH 02/14] fix(security): Sanitize fields list, group_by and order_by clause to prevent SQLi --- frappe/model/db_query.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 53a1c6c13d..74bd245fb4 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -115,8 +115,12 @@ class DatabaseQuery(object): args.fields = 'distinct ' + args.fields args.order_by = '' # TODO: recheck for alternative - query = """select %(fields)s from %(tables)s %(conditions)s - %(group_by)s %(order_by)s %(limit)s""" % args + query = """select %(fields)s + from %(tables)s + %(conditions)s + %(group_by)s + %(order_by)s + %(limit)s""" % args if self.return_query: return query @@ -240,6 +244,11 @@ class DatabaseQuery(object): _is_query(field) + if re.compile(r".*/\*.*").match(field): + frappe.throw(_('Illegal SQL Query')) + + if re.compile(r".*\s(union).*\s").match(field.lower()): + frappe.throw(_('Illegal SQL Query')) def extract_tables(self): """extract tables from fields""" @@ -688,6 +697,8 @@ class DatabaseQuery(object): if 'select' in _lower and ' from ' in _lower: frappe.throw(_('Cannot use sub-query in order by')) + if re.compile(r".*[^a-z0-9-_ ,`'\"\.\(\)].*").match(_lower): + frappe.throw(_('Illegal SQL Query')) for field in parameters.split(","): if "." in field and field.strip().startswith("`tab"): From 5d04fb4eb791cac5ae7a33f15577b2d42aa6ed2f Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Tue, 30 Jul 2019 14:09:00 +0530 Subject: [PATCH 03/14] fix(search): Reduce restrictions on field contents --- frappe/desk/reportview.py | 1 + frappe/desk/search.py | 3 ++- frappe/model/db_query.py | 13 ++++++++----- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 9654e14687..189178d878 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -68,6 +68,7 @@ def get_form_params(): # queries must always be server side data.query = None + data.strict = None return data diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 61b0cf2905..18278f7871 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -153,7 +153,8 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, order_by=order_by, ignore_permissions=ignore_permissions, reference_doctype=reference_doctype, - as_list=not as_dict) + as_list=not as_dict, + strict=False) if doctype in UNTRANSLATED_DOCTYPES: values = tuple([v for v in list(values) if re.search(txt+".*", (_(v.name) if as_dict else _(v[0])), re.IGNORECASE)]) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 74bd245fb4..f864e4f356 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -36,7 +36,7 @@ class DatabaseQuery(object): ignore_permissions=False, user=None, with_comment_count=False, join='left join', distinct=False, start=None, page_length=None, limit=None, ignore_ifnull=False, save_user_settings=False, save_user_settings_fields=False, - update=None, add_total_row=None, user_settings=None, reference_doctype=None, return_query=False): + update=None, add_total_row=None, user_settings=None, reference_doctype=None, return_query=False, strict=True): if not ignore_permissions and not frappe.has_permission(self.doctype, "read", user=user): frappe.flags.error_message = _('Insufficient Permission for {0}').format(frappe.bold(self.doctype)) raise frappe.PermissionError(self.doctype) @@ -80,6 +80,7 @@ class DatabaseQuery(object): self.update = update self.user_settings_fields = copy.deepcopy(self.fields) self.return_query = return_query + self.strict = strict # for contextual user permission check # to determine which user permission is applicable on link field of specific doctype @@ -244,11 +245,12 @@ class DatabaseQuery(object): _is_query(field) - if re.compile(r".*/\*.*").match(field): - frappe.throw(_('Illegal SQL Query')) + if self.strict: + if re.compile(r".*/\*.*").match(field): + frappe.throw(_('Illegal SQL Query')) - if re.compile(r".*\s(union).*\s").match(field.lower()): - frappe.throw(_('Illegal SQL Query')) + if re.compile(r".*\s(union).*\s").match(field.lower()): + frappe.throw(_('Illegal SQL Query')) def extract_tables(self): """extract tables from fields""" @@ -766,6 +768,7 @@ def get_list(doctype, *args, **kwargs): kwargs.pop('cmd', None) kwargs.pop('ignore_permissions', None) kwargs.pop('data', None) + kwargs.pop('strict', None) # If doctype is child table if frappe.is_table(doctype): From dca19472aae2408f41dd00878906489a256fc499 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Tue, 30 Jul 2019 17:01:59 +0530 Subject: [PATCH 04/14] fix: Enqueue personal data file generation for faster response (#7977) --- .../personal_data_download_request.py | 4 +++- .../test_personal_data_download_request.py | 23 ++++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/frappe/website/doctype/personal_data_download_request/personal_data_download_request.py b/frappe/website/doctype/personal_data_download_request/personal_data_download_request.py index d9b2856854..747b7adbbe 100644 --- a/frappe/website/doctype/personal_data_download_request/personal_data_download_request.py +++ b/frappe/website/doctype/personal_data_download_request/personal_data_download_request.py @@ -12,7 +12,9 @@ from frappe.utils.verified_command import get_signed_params class PersonalDataDownloadRequest(Document): def after_insert(self): personal_data = get_user_data(self.user) - self.generate_file_and_send_mail(personal_data) + + frappe.enqueue_doc(self.doctype, self.name, 'generate_file_and_send_mail', + queue='short', personal_data=personal_data, now=frappe.flags.in_test) def generate_file_and_send_mail(self, personal_data): """generate the file link for download""" diff --git a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py index 32b86bca0b..64d4e45660 100644 --- a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py +++ b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py @@ -21,18 +21,25 @@ class TestRequestPersonalData(unittest.TestCase): def test_file_and_email_creation(self): frappe.set_user('test_privacy@example.com') - download_request = frappe.get_doc({"doctype": 'Personal Data Download Request', 'user': 'test_privacy@example.com'}) + download_request = frappe.get_doc({ + "doctype": 'Personal Data Download Request', + 'user': 'test_privacy@example.com' + }) download_request.save(ignore_permissions=True) + frappe.set_user('Administrator') - f = frappe.get_all('File', - {'attached_to_doctype':'Personal Data Download Request', 'attached_to_name': download_request.name}, - ['*']) - self.assertEqual(len(f), 1) + file_count = frappe.db.count('File', { + 'attached_to_doctype':'Personal Data Download Request', + 'attached_to_name': download_request.name + }) - email_queue = frappe.db.sql("""SELECT * - FROM `tabEmail Queue` - ORDER BY `creation` DESC""", as_dict=True) + self.assertEqual(file_count, 1) + + email_queue = frappe.get_all('Email Queue', + fields=['message'], + order_by="creation DESC", + limit=1) self.assertTrue("Subject: Download Your Data" in email_queue[0].message) frappe.db.sql("delete from `tabEmail Queue`") From baa8f1be99ca0162c9267522024532885d49957d Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 30 Jul 2019 17:55:22 +0530 Subject: [PATCH 05/14] fix: Module check in get_desktop_settings (#8053) --- frappe/desk/moduleview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/moduleview.py b/frappe/desk/moduleview.py index ca3eef5c52..bdd1115c66 100644 --- a/frappe/desk/moduleview.py +++ b/frappe/desk/moduleview.py @@ -336,7 +336,7 @@ def get_desktop_settings(): if category in user_saved_modules_by_category: user_modules = user_saved_modules_by_category[category] user_modules_by_category[category] = [apply_user_saved_links(modules_by_name[m]) \ - for m in user_modules] + for m in user_modules if modules_by_name.get(m)] else: user_modules_by_category[category] = [apply_user_saved_links(m) \ for m in all_modules if m.get('category') == category] From 4164c62efc630879a2378630ee454263084630dc Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Wed, 31 Jul 2019 20:10:39 +0530 Subject: [PATCH 06/14] fix(doctor): Show status of scheduler in bench doctor --- frappe/commands/scheduler.py | 5 ++++- frappe/utils/doctor.py | 13 ++++++++++++- frappe/utils/scheduler.py | 20 +++++++++++++------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py index 8d1cca286d..6f51c81211 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -115,9 +115,12 @@ def set_maintenance_mode(context, state, site=None): @click.command('doctor') #Passing context always gets a site and if there is no use site it breaks @click.option('--site', help='site name') -def doctor(site=None): +@pass_context +def doctor(context, site=None): "Get diagnostic info about background workers" from frappe.utils.doctor import doctor as _doctor + if not site: + site = get_site(context) return _doctor(site=site) @click.command('show-pending-jobs') diff --git a/frappe/utils/doctor.py b/frappe/utils/doctor.py index 9f046bb8ea..e97f792b88 100644 --- a/frappe/utils/doctor.py +++ b/frappe/utils/doctor.py @@ -3,7 +3,7 @@ import frappe.utils from collections import defaultdict from rq import Worker, Connection from frappe.utils.background_jobs import get_redis_conn, get_queue, get_queue_list -from frappe.utils.scheduler import is_scheduler_disabled +from frappe.utils.scheduler import is_scheduler_disabled, is_scheduler_inactive from six import iteritems @@ -107,8 +107,19 @@ def doctor(site=None): for s in sites: frappe.init(s) frappe.connect() + if is_scheduler_disabled(): print("Scheduler disabled for", s) + + if frappe.local.conf.maintenance_mode: + print("Maintenance mode on for", s) + + if frappe.local.conf.pause_scheduler: + print("Scheduler paused for", s) + + if is_scheduler_inactive(): + print("Scheduler inactive for", s) + frappe.destroy() # TODO improve this diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index 1cf62d0cb0..605ca7c8b4 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -79,14 +79,8 @@ def enqueue_events_for_site(site, queued_jobs): try: frappe.init(site=site) - if frappe.local.conf.maintenance_mode: - return - - if frappe.local.conf.pause_scheduler: - return - frappe.connect() - if is_scheduler_disabled(): + if is_scheduler_inactive(): return enqueue_events(site=site, queued_jobs=queued_jobs) @@ -226,6 +220,18 @@ def get_enabled_scheduler_events(): return ["all", "hourly", "hourly_long", "daily", "daily_long", "weekly", "weekly_long", "monthly", "monthly_long", "cron"] +def is_scheduler_inactive(): + if frappe.local.conf.maintenance_mode: + return True + + if frappe.local.conf.pause_scheduler: + return True + + if is_scheduler_disabled(): + return True + + return False + def is_scheduler_disabled(): if frappe.conf.disable_scheduler: return True From d11159ad507259b93f597be77c18811b9319aef3 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Wed, 31 Jul 2019 20:11:54 +0530 Subject: [PATCH 07/14] fix(background-jobs): Show status of scheduler in background-jobs page --- frappe/core/page/background_jobs/background_jobs.js | 6 ++++++ frappe/core/page/background_jobs/background_jobs.py | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/frappe/core/page/background_jobs/background_jobs.js b/frappe/core/page/background_jobs/background_jobs.js index 458a8ad5ee..2bde8454a9 100644 --- a/frappe/core/page/background_jobs/background_jobs.js +++ b/frappe/core/page/background_jobs/background_jobs.js @@ -13,6 +13,12 @@ frappe.pages['background_jobs'].on_page_load = function(wrapper) { frappe.pages['background_jobs'].on_page_show = function(wrapper) { frappe.pages.background_jobs.refresh_jobs(); + frappe.call({ + method: 'frappe.core.page.background_jobs.background_jobs.get_scheduler_status', + callback: function(r) { + frappe.pages.background_jobs.page.set_indicator(...r.message); + } + }); } frappe.pages.background_jobs.refresh_jobs = function() { diff --git a/frappe/core/page/background_jobs/background_jobs.py b/frappe/core/page/background_jobs/background_jobs.py index 7fd681652a..d488ccd64a 100644 --- a/frappe/core/page/background_jobs/background_jobs.py +++ b/frappe/core/page/background_jobs/background_jobs.py @@ -7,6 +7,8 @@ import frappe from rq import Queue, Worker from frappe.utils.background_jobs import get_redis_conn from frappe.utils import format_datetime, cint +from frappe.utils.scheduler import is_scheduler_inactive +from frappe import _ colors = { 'queued': 'orange', @@ -49,3 +51,9 @@ def get_info(show_failed=False): for j in q.get_jobs()[:10]: add_job(j, q.name) return jobs + +@frappe.whitelist() +def get_scheduler_status(): + if is_scheduler_inactive(): + return [_("Inactive"), "red"] + return [_("Active"), "green"] From d4bf5b6d384630f53679c9c8ebf0dfbb36300702 Mon Sep 17 00:00:00 2001 From: Harshit <46772424+harshit-30@users.noreply.github.com> Date: Thu, 1 Aug 2019 12:55:58 +0530 Subject: [PATCH 08/14] fix: Change reset password button label (#7958) --- frappe/www/login.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/www/login.html b/frappe/www/login.html index d621ab0fbb..8c470ac6dd 100644 --- a/frappe/www/login.html +++ b/frappe/www/login.html @@ -87,7 +87,7 @@ - +
From cee2ae11dd4fdcc28140f0b99e45f1c51abb5606 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 1 Aug 2019 12:57:44 +0530 Subject: [PATCH 09/14] fix: Disable drag-n-reorder in mobile (#8067) --- frappe/public/js/frappe/views/components/DeskSection.vue | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/components/DeskSection.vue b/frappe/public/js/frappe/views/components/DeskSection.vue index 5c006920af..59a8e88baf 100644 --- a/frappe/public/js/frappe/views/components/DeskSection.vue +++ b/frappe/public/js/frappe/views/components/DeskSection.vue @@ -30,7 +30,9 @@ export default { } }, mounted() { - this.setup_sortable(); + if (!frappe.utils.is_mobile()) { + this.setup_sortable(); + } }, methods: { setup_sortable() { From 0ec41e9fabd3cced555fd0bd55332cf09ef82ce5 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 1 Aug 2019 13:04:02 +0530 Subject: [PATCH 10/14] fix: Add breadcrumbs in Pages (#8057) * fix: Add breadcrumbs in Pages * fix: Allow Report view in File --- frappe/public/js/frappe/views/file/file_view.js | 2 +- frappe/public/js/frappe/views/pageview.js | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/file/file_view.js b/frappe/public/js/frappe/views/file/file_view.js index fb01b87a61..6ee37aec3d 100644 --- a/frappe/public/js/frappe/views/file/file_view.js +++ b/frappe/public/js/frappe/views/file/file_view.js @@ -354,7 +354,7 @@ frappe.views.FileView.grid_view = frappe.get_user_settings('File').grid_view || function redirect_to_home_if_invalid_route() { const route = frappe.get_route(); - if (route[2] !== 'Home') { + if (route[2] === 'List') { // if the user somehow redirects to List/File/List // redirect back to Home frappe.set_route('List', 'File', 'Home'); diff --git a/frappe/public/js/frappe/views/pageview.js b/frappe/public/js/frappe/views/pageview.js index c31ba74285..c0149672cb 100644 --- a/frappe/public/js/frappe/views/pageview.js +++ b/frappe/public/js/frappe/views/pageview.js @@ -98,6 +98,9 @@ frappe.views.Page = Class.extend({ this.wrapper.innerHTML = this.pagedoc.content; frappe.dom.eval(this.pagedoc.__script || this.pagedoc.script || ''); frappe.dom.set_style(this.pagedoc.style || ''); + + // set breadcrumbs + frappe.breadcrumbs.add(this.pagedoc.module || null); } this.trigger_page_event('on_page_load'); From afdd485b76fb9a80a2054100d41c737192833ce0 Mon Sep 17 00:00:00 2001 From: sahil28297 <37302950+sahil28297@users.noreply.github.com> Date: Thu, 1 Aug 2019 16:59:27 +0530 Subject: [PATCH 11/14] fix(patch): empty file_url if file does not have file_url (#8074) --- frappe/patches/v12_0/fix_public_private_files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/patches/v12_0/fix_public_private_files.py b/frappe/patches/v12_0/fix_public_private_files.py index b70b905eb7..a1a2f3f6fc 100644 --- a/frappe/patches/v12_0/fix_public_private_files.py +++ b/frappe/patches/v12_0/fix_public_private_files.py @@ -7,7 +7,7 @@ def execute(): filters={'is_folder': 0}) for file in files: - file_url = file.file_url + file_url = file.file_url or "" if file.is_private: if not file_url.startswith('/private/files/'): generate_file(file.name) @@ -32,4 +32,4 @@ def generate_file(file_name): except IOError: pass except Exception as e: - print(e) \ No newline at end of file + print(e) From c33f7f1e45b6244857b4fd4a6a8d59d2bd072fdb Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 2 Aug 2019 12:42:00 +0530 Subject: [PATCH 12/14] fix: Pick sitemap host from request (#8071) --- frappe/utils/data.py | 12 +++++++++--- frappe/www/sitemap.py | 7 ++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index ae74996247..9276a24d0a 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -714,9 +714,10 @@ def get_url(uri=None, full_address=False): return uri if not host_name: - if hasattr(frappe.local, "request") and frappe.local.request and frappe.local.request.host: - protocol = 'https://' if 'https' == frappe.get_request_header('X-Forwarded-Proto', "") else 'http://' - host_name = protocol + frappe.local.request.host + request_host_name = get_host_name_from_request() + + if request_host_name: + host_name = request_host_name elif frappe.local.site: protocol = 'http://' @@ -753,6 +754,11 @@ def get_url(uri=None, full_address=False): return url +def get_host_name_from_request(): + if hasattr(frappe.local, "request") and frappe.local.request and frappe.local.request.host: + protocol = 'https://' if 'https' == frappe.get_request_header('X-Forwarded-Proto', "") else 'http://' + return protocol + frappe.local.request.host + def url_contains_port(url): parts = url.split(':') return len(parts) > 2 diff --git a/frappe/www/sitemap.py b/frappe/www/sitemap.py index df51815329..8b93270ab5 100644 --- a/frappe/www/sitemap.py +++ b/frappe/www/sitemap.py @@ -15,7 +15,12 @@ base_template_path = "templates/www/sitemap.xml" def get_context(context): """generate the sitemap XML""" - host = get_request_site_address() + + # the site might be accessible from multiple host_names + # for e.g gadgets.erpnext.com and gadgetsinternational.com + # so it should be picked from the request + host = frappe.utils.get_host_name_from_request() + links = [] for route, page in iteritems(get_pages()): if page.sitemap: From 6493463c89c63b07ed9a8364447c09ece5f00a03 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 2 Aug 2019 12:58:34 +0530 Subject: [PATCH 13/14] fix: Blacklist iframe from sanitize_html (#7982) --- frappe/utils/html_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/html_utils.py b/frappe/utils/html_utils.py index 2c3364660b..1dd06ffb3a 100644 --- a/frappe/utils/html_utils.py +++ b/frappe/utils/html_utils.py @@ -58,7 +58,7 @@ def sanitize_html(html, linkify=False): return html tags = (acceptable_elements + svg_elements + mathml_elements - + ["html", "head", "meta", "link", "body", "iframe", "style", "o:p"]) + + ["html", "head", "meta", "link", "body", "style", "o:p"]) attributes = {"*": acceptable_attributes, 'svg': svg_attributes} styles = bleach_whitelist.all_styles strip_comments = False From 6ab1e72ff781faa6348f48a27b72265f58eed247 Mon Sep 17 00:00:00 2001 From: Sahil Khan Date: Fri, 2 Aug 2019 14:46:27 +0550 Subject: [PATCH 14/14] bumped to version 12.0.4 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index e888379ecd..afd6036682 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -23,7 +23,7 @@ if sys.version[0] == '2': reload(sys) sys.setdefaultencoding("utf-8") -__version__ = '12.0.3' +__version__ = '12.0.4' __title__ = "Frappe Framework" local = Local()