From 61b2b2598152d3f29ff70becc3e32b4748a91a5f Mon Sep 17 00:00:00 2001 From: Achilles Rasquinha Date: Wed, 13 Dec 2017 16:25:06 +0530 Subject: [PATCH 1/4] [FIX] Fix XSS (#4560) * [FIX] login xss * [FIX] Client Side Sanitization * moved from frappe to frappe.utils * added strategies * removed console.log * fixed codacy * XSS sanitization at login * moved to common js - xss_sanitize --- frappe/public/js/frappe/misc/common.js | 33 ++++++++++++++++++++++++ frappe/templates/includes/login/login.js | 4 +-- frappe/website/js/website.js | 1 + 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/misc/common.js b/frappe/public/js/frappe/misc/common.js index 264ca8aae0..97a91e68da 100644 --- a/frappe/public/js/frappe/misc/common.js +++ b/frappe/public/js/frappe/misc/common.js @@ -255,3 +255,36 @@ frappe.is_mobile = function() { return $(document).width() < 768; } +frappe.utils.xss_sanitise = function (string, options) { + // Reference - https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet + let sanitised = string; // un-sanitised string. + const DEFAULT_OPTIONS = { + strategies: ['html', 'js'] // use all strategies. + } + const HTML_ESCAPE_MAP = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '/': '/' + }; + const REGEX_SCRIPT = /)<[^<]*)*<\/script>/gi; // used in jQuery 1.7.2 src/ajax.js Line 14 + options = Object.assign({ }, DEFAULT_OPTIONS, options); // don't deep copy, immutable beauty. + + // Rule 1 + if ( options.strategies.includes('html') ) { + // By far, the best thing that has ever happened to JS - Object.keys + Object.keys(HTML_ESCAPE_MAP).map((char, escape) => { + const regex = new RegExp(char, "g"); + sanitised = sanitised.replace(regex, escape); + }); + } + + // Rule 3 - TODO: Check event handlers? + if ( options.strategies.includes('js') ) { + sanitised = sanitised.replace(REGEX_SCRIPT, ""); + } + + return sanitised; +} \ No newline at end of file diff --git a/frappe/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index 69e1199254..f9a5bc44b8 100644 --- a/frappe/templates/includes/login/login.js +++ b/frappe/templates/includes/login/login.js @@ -17,8 +17,8 @@ login.bind_events = function() { event.preventDefault(); var args = {}; args.cmd = "login"; - args.usr = ($("#login_email").val() || "").trim(); - args.pwd = $("#login_password").val(); + args.usr = frappe.utils.xss_sanitise(($("#login_email").val() || "").trim()); + args.pwd = frappe.utils.xss_sanitise($("#login_password").val()); args.device = "desktop"; if(!args.usr || !args.pwd) { frappe.msgprint("{{ _("Both login and password required") }}"); diff --git a/frappe/website/js/website.js b/frappe/website/js/website.js index 0d50c85b9c..b85a45f449 100644 --- a/frappe/website/js/website.js +++ b/frappe/website/js/website.js @@ -185,6 +185,7 @@ $.extend(frappe, { if($.isArray(html)) { html = html.join("
") } + return frappe.get_modal(title || "Message", html).modal("show"); }, send_message: function(opts, btn) { From d5dcc0b98f9820bed35452bf5c065b3b0256f6c9 Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Thu, 14 Dec 2017 10:27:05 +0100 Subject: [PATCH 2/4] Issue 4616 (#4617) * fail silently when key not found * PEP 8: spacing, docstrings * codacy --- frappe/utils/global_search.py | 92 ++++++++++++++++++++++++++--------- 1 file changed, 70 insertions(+), 22 deletions(-) diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index 84c3308e96..5811d78057 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -10,8 +10,12 @@ from frappe.utils import cint, strip_html_tags from frappe.model.base_document import get_controller from six import text_type + def setup_global_search_table(): - '''Creates __global_seach table''' + """ + Creates __global_seach table + :return: + """ if not '__global_search' in frappe.db.get_tables(): frappe.db.sql('''create table __global_search( doctype varchar(100), @@ -26,12 +30,21 @@ def setup_global_search_table(): ENGINE=MyISAM CHARACTER SET=utf8mb4''') + def reset(): - '''Deletes all data in __global_search''' + """ + Deletes all data in __global_search + :return: + """ frappe.db.sql('delete from __global_search') + def get_doctypes_with_global_search(with_child_tables=True): - '''Return doctypes with global search fields''' + """ + Return doctypes with global search fields + :param with_child_tables: + :return: + """ def _get(): global_search_doctypes = [] filters = {} @@ -43,17 +56,25 @@ def get_doctypes_with_global_search(with_child_tables=True): global_search_doctypes.append(d) installed_apps = frappe.get_installed_apps() + module_app = frappe.local.module_app + + doctypes = [ + d.name for d in global_search_doctypes + if module_app.get(frappe.scrub(d.module)) + and module_app[frappe.scrub(d.module)] in installed_apps + ] - doctypes = [d.name for d in global_search_doctypes - if frappe.local.module_app[frappe.scrub(d.module)] in installed_apps] return doctypes return frappe.cache().get_value('doctypes_with_global_search', _get) + def rebuild_for_doctype(doctype): - '''Rebuild entries of doctype's documents in __global_search on change of - searchable fields - :param doctype: Doctype ''' + """ + Rebuild entries of doctype's documents in __global_search on change of + searchable fields + :param doctype: Doctype + """ def _get_filters(): filters = frappe._dict({ "docstatus": ["!=", 2] }) @@ -127,6 +148,7 @@ def rebuild_for_doctype(doctype): if all_contents: insert_values_for_multiple_docs(all_contents) + def delete_global_search_records_for_doctype(doctype): frappe.db.sql(''' delete @@ -134,6 +156,7 @@ def delete_global_search_records_for_doctype(doctype): where doctype = %s''', doctype, as_dict=True) + def get_selected_fields(meta, global_search_fields): fieldnames = [df.fieldname for df in global_search_fields] if meta.istable==1: @@ -146,6 +169,7 @@ def get_selected_fields(meta, global_search_fields): return fieldnames + def get_children_data(doctype, meta): """ Get all records from all the child tables of a doctype @@ -182,6 +206,7 @@ def get_children_data(doctype, meta): return all_children, child_search_fields + def insert_values_for_multiple_docs(all_contents): values = [] for content in all_contents: @@ -201,9 +226,11 @@ def insert_values_for_multiple_docs(all_contents): def update_global_search(doc): - '''Add values marked with `in_global_search` to - `frappe.flags.update_global_search` from given doc - :param doc: Document to be added to global search''' + """ + Add values marked with `in_global_search` to + `frappe.flags.update_global_search` from given doc + :param doc: Document to be added to global search + """ if doc.docstatus > 1 or (doc.meta.has_field("enabled") and not doc.get("enabled")) \ or doc.get("disabled"): @@ -235,6 +262,7 @@ def update_global_search(doc): published=published, title=doc.get_title(), route=doc.get('route'))) enqueue_global_search() + def enqueue_global_search(): if frappe.flags.update_global_search: try: @@ -246,21 +274,32 @@ def enqueue_global_search(): frappe.flags.update_global_search = [] + def get_formatted_value(value, field): - '''Prepare field from raw data''' + """ + Prepare field from raw data + :param value: + :param field: + :return: + """ from six.moves.html_parser import HTMLParser - if(getattr(field, 'fieldtype', None) in ["Text", "Text Editor"]): + if getattr(field, 'fieldtype', None) in ["Text", "Text Editor"]: h = HTMLParser() value = h.unescape(value) value = (re.subn(r'<[\s]*(script|style).*?(?s)', '', text_type(value))[0]) value = ' '.join(value.split()) return field.label + " : " + strip_html_tags(text_type(value)) + def sync_global_search(flags=None): - '''Add values from `flags` (frappe.flags.update_global_search) to __global_search. - This is called internally at the end of the request.''' + """ + Add values from `flags` (frappe.flags.update_global_search) to __global_search. + This is called internally at the end of the request. + :param flags: + :return: + """ if not flags: flags = frappe.flags.update_global_search @@ -278,10 +317,13 @@ def sync_global_search(flags=None): frappe.flags.update_global_search = [] + def delete_for_document(doc): - '''Delete the __global_search entry of a document that has - been deleted - :param doc: Deleted document''' + """ + Delete the __global_search entry of a document that has + been deleted + :param doc: Deleted document + """ frappe.db.sql(''' delete @@ -290,13 +332,16 @@ def delete_for_document(doc): doctype = %s and name = %s''', (doc.doctype, doc.name), as_dict=True) + @frappe.whitelist() def search(text, start=0, limit=20, doctype=""): - '''Search for given text in __global_search + """ + Search for given text in __global_search :param text: phrase to be searched :param start: start results at, default 0 :param limit: number of results to return, default 20 - :return: Array of result objects''' + :return: Array of result objects + """ text = "+" + text + "*" if not doctype: @@ -328,13 +373,16 @@ def search(text, start=0, limit=20, doctype=""): return results + @frappe.whitelist(allow_guest=True) def web_search(text, start=0, limit=20): - '''Search for given text in __global_search where published = 1 + """ + Search for given text in __global_search where published = 1 :param text: phrase to be searched :param start: start results at, default 0 :param limit: number of results to return, default 20 - :return: Array of result objects''' + :return: Array of result objects + """ text = "+" + text + "*" results = frappe.db.sql(''' From e44ee0ae7f915114eeed74ee4fd309b923354a57 Mon Sep 17 00:00:00 2001 From: Shreya Shah Date: Fri, 15 Dec 2017 12:15:58 +0530 Subject: [PATCH 3/4] disabled user not allowed to reset password (#4630) --- frappe/core/doctype/user/user.py | 2 ++ frappe/templates/includes/login/login.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 7a224719f8..833beb5cc9 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -769,6 +769,8 @@ def reset_password(user): try: user = frappe.get_doc("User", user) + if not user.enabled: + return 'disabled' user.validate_reset_password() user.reset_password(send_email=True) diff --git a/frappe/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index f9a5bc44b8..d02d449c38 100644 --- a/frappe/templates/includes/login/login.js +++ b/frappe/templates/includes/login/login.js @@ -184,6 +184,8 @@ login.login_handlers = (function() { login.set_indicator("{{ _("Not a valid user") }}", 'red'); } else if (data.message=='not allowed') { login.set_indicator("{{ _("Not Allowed") }}", 'red'); + } else if (data.message=='disabled') { + login.set_indicator("{{ _("Not Allowed: Disabled User") }}", 'red'); } else { login.set_indicator("{{ _("Instructions Emailed") }}", 'green'); } From b41bc5c1f96fa54cef5505058477ac3ad05adff5 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Fri, 15 Dec 2017 13:22:54 +0600 Subject: [PATCH 4/4] bumped to version 9.2.24 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index a0fdf7c6c8..8fb2a1bb3a 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__ = '9.2.23' +__version__ = '9.2.24' __title__ = "Frappe Framework" local = Local()