Skip to content

Commit

Permalink
Prevent non http/https links from being unescaped
Browse files Browse the repository at this point in the history
Our permissive HTML escaping is preserving some HTML tags, which includes basic
link tags (a tag with href and optional target attributes). Previous versions
were not inspecting the value of href, which made it possible to add links with
e.g. a "javascript:" protocol. This opened some XSS attack vectors.

After this change it is only possible to link to http and https protocols. All
other links will not be unescaped.

Change-Id: If639df20428e46d5bdc7ef14dec659babd89f86d
  • Loading branch information
LarsMichelsen committed Oct 20, 2020
1 parent fdc83fe commit 87ceb96
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
6 changes: 6 additions & 0 deletions cmk/gui/htmllib.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import json
import abc
import pprint
import urlparse
from contextlib import contextmanager

import six
Expand Down Expand Up @@ -163,6 +164,11 @@ def escape_text(self, text):
text = self._unescaper_text.sub(r'<\1\2>', text)
for a_href in self._a_href.finditer(text):
href = a_href.group(1)

parsed = urlparse.urlparse(href)
if parsed.scheme != "" and parsed.scheme not in ["http", "https"]:
continue # Do not unescape links containing disallowed URLs

target = a_href.group(2)

if target:
Expand Down
10 changes: 8 additions & 2 deletions tests/unit/cmk/gui/test_htmllib_Escaper.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,14 @@ def test_unescape_attribute(inp, out):
"<a href=\"xyz\">abc</a>&lt;script&gt;alert(1)&lt;/script&gt;<a href=\"xyz\">abc</a>",
),
("&nbsp;", None),
# At the moment also javascript URLs are accepted. This will be refused in the next step
("<a href=\"javascript:alert(1)\">abc</a>", None),
# Only http/https are allowed as schemes
("<a href=\"http://checkmk.com/\">abc</a>", None),
("<a href=\"https://checkmk.com/\">abc</a>", None),
("<a href=\"HTTP://CHECKMK.COM/\">abc</a>", None),
("<a href=\"ftp://checkmk.com/\">abc</a>",
"&lt;a href=&quot;ftp://checkmk.com/&quot;&gt;abc</a>"),
("<a href=\"javascript:alert(1)\">abc</a>",
"&lt;a href=&quot;javascript:alert(1)&quot;&gt;abc</a>"),
])
def test_escape_text(inp, out):
if out is None:
Expand Down

0 comments on commit 87ceb96

Please sign in to comment.