18 Sep
2018
18 Sep
'18
5:30 p.m.
Module: check_mk
Branch: master
Commit: 5fe3b85224edaf8e1fbe80081976ff3125335aaf
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=5fe3b85224edaf…
Author: Lars Michelsen <lm(a)mathias-kettner.de>
Date: Fri Sep 14 08:32:08 2018 +0200
6612 SEC Fixed possible reflected XSS using back URLs in view editor
The parameter back of the following requests is vulnerable to reflected XSS.
This vulnerability affects the create/modify view page and requires at least
guest privileges. The victim has to click on the back button to trigger the
injected code.
Change-Id: I56d31e7e884cab576096496ab3676361e653d10d
---
.werks/6612 | 13 +++++++++++++
cmk/gui/main.py | 36 ++++++++++++------------------------
cmk/gui/utils.py | 21 +++++++++++++++++++++
cmk/gui/views.py | 2 ++
cmk/gui/visuals.py | 6 ++++++
5 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/.werks/6612 b/.werks/6612
new file mode 100644
index 0000000..2d536b2
--- /dev/null
+++ b/.werks/6612
@@ -0,0 +1,13 @@
+Title: Fixed possible reflected XSS using back URLs in view editor
+Level: 1
+Component: multisite
+Compatible: compat
+Edition: cre
+Version: 1.6.0i1
+Date: 1536906650
+Class: security
+
+The parameter back of the following requests is vulnerable to reflected XSS.
+This vulnerability affects the create/modify view page and requires at least
+guest privileges. The victim has to click on the back button to trigger the
+injected code.
diff --git a/cmk/gui/main.py b/cmk/gui/main.py
index 08a3b3d..341c5db 100644
--- a/cmk/gui/main.py
+++ b/cmk/gui/main.py
@@ -24,36 +24,14 @@
# to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
# Boston, MA 02110-1301 USA.
-import urlparse
-import re
-
import cmk.gui.pages
import cmk.gui.config as config
+import cmk.gui.utils as utils
from cmk.gui.i18n import _
from cmk.gui.globals import html
@cmk.gui.pages.register("index")
def page_index():
- default_start_url = config.user.get_attribute("start_url") or
config.start_url
- start_url = html.var_utf8("start_url", default_start_url).strip()
-
- # Prevent redirecting to absolute URL which could be used to redirect
- # users to compromised pages.
- # Also prevent using of "javascript:" URLs which could used to inject code
- parsed = urlparse.urlparse(start_url)
-
- # Don't allow the user to set a URL scheme
- if parsed.scheme != "":
- start_url = default_start_url
-
- # Don't allow the user to set a network location
- if parsed.netloc != "":
- start_url = default_start_url
-
- # Don't allow bad characters in path
- if not re.match(r"[/a-z0-9_\.-]*$", parsed.path):
- start_url = default_start_url
-
html.write('<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01
Frameset//EN" "http://www.w3.org/TR/html4/frameset.dtd">\n'
'<html><head>\n')
html.default_html_headers()
@@ -64,4 +42,14 @@ def page_index():
<frame src="%s" name="main" noresize>
</frameset>
</html>
-""" % (html.attrencode(config.get_page_heading()),
html.attrencode(start_url)))
+""" % (html.attrencode(config.get_page_heading()),
html.attrencode(_get_start_url())))
+
+
+def _get_start_url():
+ start_url = html.var_utf8("start_url",
config.user.get_attribute("start_url") or config.start_url).strip()
+ # Prevent redirecting to absolute URL which could be used to redirect
+ # users to compromised pages.
+ if utils.is_allowed_url(start_url):
+ return start_url
+ else:
+ return "dashboard.py"
diff --git a/cmk/gui/utils.py b/cmk/gui/utils.py
index c67c4a3..fc29cc3 100644
--- a/cmk/gui/utils.py
+++ b/cmk/gui/utils.py
@@ -33,6 +33,7 @@ import os
import re
import uuid
import marshal
+import urlparse
import cmk.paths
@@ -86,6 +87,26 @@ def cmp_version(a, b):
return cmp(aa, bb)
+def is_allowed_url(url):
+ """Checks whether or not the given URL is a URL it is allowed to
redirect the user to"""
+ # Also prevent using of "javascript:" URLs which could used to inject code
+ parsed = urlparse.urlparse(url)
+
+ # Don't allow the user to set a URL scheme
+ if parsed.scheme != "":
+ return False
+
+ # Don't allow the user to set a network location
+ if parsed.netloc != "":
+ return False
+
+ # Don't allow bad characters in path
+ if not re.match(r"[/a-z0-9_\.-]*$", parsed.path):
+ return False
+
+ return True
+
+
# TODO: Remove this helper function. Replace with explicit checks and covnersion
# in using code.
def savefloat(f):
diff --git a/cmk/gui/views.py b/cmk/gui/views.py
index 28a8b45..535115a 100644
--- a/cmk/gui/views.py
+++ b/cmk/gui/views.py
@@ -310,6 +310,8 @@ def page_create_view(next_url = None):
html.header(_('Create View'), stylesheets=["pages"])
html.begin_context_buttons()
back_url = html.var("back", "")
+ if not utils.is_allowed_url(back_url):
+ back_url = "edit_views.py"
html.context_button(_("Back"), back_url or "edit_views.py",
"back")
html.end_context_buttons()
diff --git a/cmk/gui/visuals.py b/cmk/gui/visuals.py
index ce1688a..d19a6d2 100644
--- a/cmk/gui/visuals.py
+++ b/cmk/gui/visuals.py
@@ -540,6 +540,8 @@ def page_create_visual(what, info_keys, next_url = None):
html.header(_('Create %s') % title, stylesheets=["pages"])
html.begin_context_buttons()
back_url = html.var("back", "")
+ if not utils.is_allowed_url(back_url):
+ back_url = "edit_%s.py" % what
html.context_button(_("Back"), back_url or "edit_%s.py" % what,
"back")
html.end_context_buttons()
@@ -735,6 +737,8 @@ def page_edit_visual(what, all_visuals, custom_field_handler = None,
html.header(title, stylesheets=["pages", "views",
"status", "bi"])
html.begin_context_buttons()
back_url = html.var("back", "")
+ if not utils.is_allowed_url(back_url):
+ back_url = "edit_%s.py" % what
html.context_button(_("Back"), back_url or "edit_%s.py" % what,
"back")
# Extra buttons to sub modules. These are used for things to edit about
@@ -856,6 +860,8 @@ def page_edit_visual(what, all_visuals, custom_field_handler = None,
back = html.var('back')
if not back:
back = 'edit_%s.py' % what
+ if not utils.is_allowed_url(back):
+ back = 'edit_%s.py' % what
if html.check_transaction():
all_visuals[(owner_user_id, visual["name"])] = visual