[checkmk-commits] 7067 SEC mail: escape notification context

Module: check_mk Branch: master Commit: d1ad475ab4ce3304bfd663806d7315a73d60fc7b URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=d1ad475ab4ce33… Author: Tom Baerwinkel &lt;tb(a)mathias-kettner.de&gt; Date: Wed Mar 20 15:34:10 2019 +0100 7067 SEC mail: escape notification context The notification context which contains e.g. the host name and the service description is now escaped properly, i.e. '<' is replaced by '&lt;' and so on. Only the HTML section which can be specified in the options of a notification rule is not escaped. Change-Id: I80fe51fb3866527286f474aa569d13b9940e1561 --- .werks/7067 | 14 ++++++++++++++ cmk/notification_plugins/mail.py | 3 ++- cmk/notification_plugins/utils.py | 11 +++++++++-- tests/unit/cmk/notifications/test_utils.py | 27 +++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/.werks/7067 b/.werks/7067 new file mode 100644 index 0000000..433c64f --- /dev/null +++ b/.werks/7067 @@ -0,0 +1,14 @@ +Title: mail: escape notification context +Level: 1 +Component: notifications +Compatible: compat +Edition: cre +Version: 1.6.0i1 +Date: 1553092068 +Class: security + +The notification context which contains e.g. the host name +and the service description is now escaped properly, i.e. +'<' is replaced by '&lt;' and so on. Only the HTML section +which can be specified in the options of a notification +rule is not escaped. diff --git a/cmk/notification_plugins/mail.py b/cmk/notification_plugins/mail.py index fe10317..bb84828 100644 --- a/cmk/notification_plugins/mail.py +++ b/cmk/notification_plugins/mail.py @@ -915,6 +915,7 @@ def main(): hosts = set([]) for context in contexts: context.update(parameters) + utils.html_escape_context(context) txt, html, att = construct_content(context) content_txt += txt content_html += html @@ -926,10 +927,10 @@ def main(): # Use the single context subject in case there is only one context in the bulk if len(contexts) > 1: subject = utils.get_bulk_notification_subject(contexts, hosts) - else: # gather all options from env context = utils.collect_context() + utils.html_escape_context(context) content_txt, content_html, attachments = construct_content(context) mailto = context['CONTACTEMAIL'] subject = context['SUBJECT'] diff --git a/cmk/notification_plugins/utils.py b/cmk/notification_plugins/utils.py index 209584f..4089785 100644 --- a/cmk/notification_plugins/utils.py +++ b/cmk/notification_plugins/utils.py @@ -94,7 +94,14 @@ def cmk_links(context): return None, None -def replace_variable_context(template, context): +def html_escape_context(context): + unescaped_variables = {'PARAMETER_INSERT_HTML_SECTION'} + for variable, value in context.iteritems(): + if variable not in unescaped_variables: + context[variable] = html_escape(value) + + +def add_debug_output(template, context): ascii_output = "" html_output = "<table class=context>\n" elements = context.items() @@ -114,7 +121,7 @@ def substitute_context(template, context): # Debugging of variables. Create content only on demand if "$CONTEXT_ASCII$" in template or "$CONTEXT_HTML$" in template: - template = replace_variable_context(template, context) + template = add_debug_output(template, context) if re.search(r"\$[A-Z_][A-Z_0-9]*\$", template): # Second pass to replace nested variables inside e.g. SERVICENOTESURL diff --git a/tests/unit/cmk/notifications/test_utils.py b/tests/unit/cmk/notifications/test_utils.py index e0bfbd7..955e3d8 100644 --- a/tests/unit/cmk/notifications/test_utils.py +++ b/tests/unit/cmk/notifications/test_utils.py @@ -88,3 +88,30 @@ def test_get_bulk_notification_subject(context, hosts, result): def test_api_endpoint_url(monkeypatch, value, result): monkeypatch.setattr('cmk.utils.password_store.extract', lambda x: 'http://secret.host') assert utils.retrieve_from_passwordstore(value) == result + + +(a)pytest.mark.parametrize( + "input_context,expected_context", + [ + # If not explicitly allowed as unescaped... + ( + { + 'PARAMETER_INSERT_HTML_SECTION': '<h1>Important</h1>' + }, + { + 'PARAMETER_INSERT_HTML_SECTION': '<h1>Important</h1>' + }, + ), + # ... all variables should be escaped + ( + { + 'SERVICEOUTPUT': '<h1>Important</h1>' + }, + { + 'SERVICEOUTPUT': '&lt;h1&gt;Important&lt;/h1&gt;' + }, + ), + ]) +def test_escape_context(input_context, expected_context): + utils.html_escape_context(input_context) + assert input_context == expected_context