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: I6e029ecc52f3dd3fc1f213c7f809332e3e49b3ee
  • Loading branch information
LarsMichelsen committed Oct 21, 2020
1 parent c00f450 commit e7fd8e4
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
6 changes: 6 additions & 0 deletions cmk/gui/escaping.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from html import escape as html_escape
import re
from typing import Union
from urllib.parse import urlparse

from six import ensure_str

Expand Down Expand Up @@ -113,6 +114,11 @@ def escape_text(text: EscapableEntity) -> str:
text = _UNESCAPER_TEXT.sub(r'<\1\2>', text)
for a_href in _A_HREF.finditer(text):
href = a_href.group(1)

parsed = 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 @@ -72,8 +72,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 e7fd8e4

Please sign in to comment.