[checkmk-commits] 5427 SEC Fixed different XSS issues triggerd from BI aggregation/ rule titles/descriptions

Module: check_mk Branch: master Commit: d788d1911002db5efd623e43f522e4471d1aa31a URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=d788d1911002db… Author: Lars Michelsen &lt;lm(a)mathias-kettner.de&gt; Date: Fri Oct 27 16:18:23 2017 +0200 5427 SEC Fixed different XSS issues triggerd from BI aggregation/rule titles/descriptions Different fields in the BI configuration (titles, ...) could be used to inject JS code into the WATO dialogs and the BI status views. This could be triggered by users with permission to administrate WATO. Change-Id: I5603d0464da058d73a2077ac46c2dbd1824dd9f8 --- .werks/5427 | 13 +++++++++++++ web/htdocs/bi.py | 4 ++-- web/htdocs/table.py | 5 +++++ web/plugins/views/bi.py | 6 +++--- web/plugins/wato/bi.py | 36 ++++++++++++++++++------------------ 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/.werks/5427 b/.werks/5427 new file mode 100644 index 0000000..9d7ad17 --- /dev/null +++ b/.werks/5427 @@ -0,0 +1,13 @@ +Title: Fixed different XSS issues triggerd from BI aggregation/rule titles/descriptions +Level: 1 +Component: bi +Class: security +Compatible: compat +Edition: cre +State: unknown +Version: 1.5.0i1 +Date: 1509112079 + +Different fields in the BI configuration (titles, ...) could be used to inject JS code +into the WATO dialogs and the BI status views. This could be triggered by users with +permission to administrate WATO. diff --git a/web/htdocs/bi.py b/web/htdocs/bi.py index d8b0aed..e61cc82 100644 --- a/web/htdocs/bi.py +++ b/web/htdocs/bi.py @@ -2931,7 +2931,7 @@ def render_tree_foldable(row, boxes, omit_root, expansion_level, only_problems, if is_leaf: h += aggr_render_leaf(tree, show_host, bare = True) # .replace(" ", "&nbsp;") else: - h += tree[2]["title"].replace(" ", "&nbsp;") + h += html.attrencode(tree[2]["title"].replace(" ", "&nbsp;")) h += '</span> ' if not is_leaf and not omit_content: @@ -2957,7 +2957,7 @@ def render_tree_foldable(row, boxes, omit_root, expansion_level, only_problems, css_class = "open" if is_open else "closed" - h += aggr_render_node(tree, tree[2]["title"], show_host, + h += aggr_render_node(tree, html.attrencode(tree[2]["title"]), show_host, mousecode=mc, img_class=css_class) if not is_empty: h += '<ul id="%d:%s" class="subtree %s">' % \ diff --git a/web/htdocs/table.py b/web/htdocs/table.py index 1082814..8e72274 100644 --- a/web/htdocs/table.py +++ b/web/htdocs/table.py @@ -50,6 +50,11 @@ def row(*posargs, **kwargs): tables[-1].row(*posargs, **kwargs) +def text_cell(*posargs, **kwargs): + assert tables + tables[-1].text_cell(*posargs, **kwargs) + + def cell(*posargs, **kwargs): assert tables tables[-1].cell(*posargs, **kwargs) diff --git a/web/plugins/views/bi.py b/web/plugins/views/bi.py index e6dcafb..539d90a 100644 --- a/web/plugins/views/bi.py +++ b/web/plugins/views/bi.py @@ -161,14 +161,14 @@ multisite_painters["aggr_group"] = { "title" : _("Aggregation group"), "short" : _("Group"), "columns" : [ "aggr_group" ], - "paint" : lambda row: ("", row["aggr_group"]) + "paint" : lambda row: ("", html.attrencode(row["aggr_group"])) } multisite_painters["aggr_name"] = { "title" : _("Aggregation name"), "short" : _("Aggregation"), "columns" : [ "aggr_name" ], - "paint" : lambda row: ("", row["aggr_name"]) + "paint" : lambda row: ("", html.attrencode(row["aggr_name"])) } multisite_painters["aggr_output"] = { @@ -272,7 +272,7 @@ def paint_aggr_tree_ltr(row, mirror): if not node[2].get("hidden"): leaves += gen_table(node, height - 1, show_host) h = '<div class="aggr tree">' \ - + bi.aggr_render_node(tree, tree[2]["title"], show_host) + "</div>" + + bi.aggr_render_node(tree, html.attrencode(tree[2]["title"]), show_host) + "</div>" if leaves: leaves[0][2].append((len(leaves), h)) return leaves diff --git a/web/plugins/wato/bi.py b/web/plugins/wato/bi.py index 3bcfd17..47e6a59 100644 --- a/web/plugins/wato/bi.py +++ b/web/plugins/wato/bi.py @@ -832,7 +832,7 @@ class ModeBI(WatoMode): if not sub_rule_ids: html.open_li() html.open_a(href=edit_url) - html.write(title) + html.write_text(title) html.close_a() html.close_li() else: @@ -910,12 +910,12 @@ class ModeBIPacks(ModeBI): html.icon_button(delete_url, _("Delete this BI pack"), "delete") rules_url = html.makeuri_contextless([("mode", "bi_rules"), ("pack", pack_id)]) html.icon_button(rules_url, _("View and edit the rules and aggregations in this BI pack"), "bi_rules") - table.cell(_("ID"), pack_id) - table.cell(_("Title"), pack["title"]) - table.cell(_("Public"), pack["public"] and _("Yes") or _("No")) - table.cell(_("Aggregations"), len(pack["aggregations"]), css="number") - table.cell(_("Rules"), len(pack["rules"]), css="number") - table.cell(_("Contact groups"), HTML(", ").join(map(self._render_contact_group, pack["contact_groups"]))) + table.text_cell(_("ID"), pack_id) + table.text_cell(_("Title"), pack["title"]) + table.text_cell(_("Public"), pack["public"] and _("Yes") or _("No")) + table.text_cell(_("Aggregations"), len(pack["aggregations"]), css="number") + table.text_cell(_("Rules"), len(pack["rules"]), css="number") + table.text_cell(_("Contact groups"), HTML(", ").join(map(self._render_contact_group, pack["contact_groups"]))) table.end() @@ -1165,22 +1165,22 @@ class ModeBIAggregations(ModeBI): delete_url = html.makeactionuri([("_del_aggr", aggregation_id)]) html.icon_button(delete_url, _("Delete this aggregation"), "delete") - table.cell(_("Nr."), aggregation_id+1, css="number") - table.cell("", css="buttons") + table.text_cell(_("Nr."), aggregation_id+1, css="number") + table.text_cell("", css="buttons") if aggregation["disabled"]: html.icon(_("This aggregation is currently disabled."), "disabled") if aggregation["single_host"]: html.icon(_("This aggregation covers only data from a single host."), "host") - table.cell(_("Groups"), ", ".join(aggregation["groups"])) + table.text_cell(_("Groups"), ", ".join(aggregation["groups"])) ruleid, description = self.rule_called_by_node(aggregation["node"]) edit_url = html.makeuri([("mode", "bi_edit_rule"), ("pack", self._pack_id), ("id", ruleid)]) table.cell(_("Rule Tree"), css="bi_rule_tree") self.render_aggregation_rule_tree(aggregation) - table.cell(_("Note"), description) + table.text_cell(_("Note"), description) table.end() @@ -1387,12 +1387,12 @@ class ModeBIRules(ModeBI): else: html.empty_icon_button() - table.cell(_("Level"), level or "", css="number") - table.cell(_("ID"), html.render_a(rule_id, edit_url)) - table.cell(_("Parameters"), " ".join(rule["params"])) - table.cell(_("Title"), rule["title"]) - table.cell(_("Aggregation"), "/".join([rule["aggregation"][0]] + map(str, rule["aggregation"][1]))) - table.cell(_("Nodes"), len(rule["nodes"]), css="number") + table.text_cell(_("Level"), level or "", css="number") + table.text_cell(_("ID"), html.render_a(rule_id, edit_url)) + table.text_cell(_("Parameters"), " ".join(rule["params"])) + table.text_cell(_("Title"), rule["title"]) + table.text_cell(_("Aggregation"), "/".join([rule["aggregation"][0]] + map(str, rule["aggregation"][1]))) + table.text_cell(_("Nodes"), len(rule["nodes"]), css="number") table.cell(_("Used by")) have_this = set([]) for (aggr_id, aggregation) in aggregations_that_use_rule.get(rule_id, []): @@ -1402,7 +1402,7 @@ class ModeBIRules(ModeBI): html.a(self.aggregation_title(aggregation), href=aggr_url) html.br() have_this.add(aggr_id) - table.cell(_("Comment"), rule.get("comment", "")) + table.text_cell(_("Comment"), rule.get("comment", "")) table.end()