Skip to content

Commit e7fd8e4

Browse files
committed
Prevent non http/https links from being unescaped
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
1 parent c00f450 commit e7fd8e4

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

cmk/gui/escaping.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from html import escape as html_escape
88
import re
99
from typing import Union
10+
from urllib.parse import urlparse
1011

1112
from six import ensure_str
1213

@@ -113,6 +114,11 @@ def escape_text(text: EscapableEntity) -> str:
113114
text = _UNESCAPER_TEXT.sub(r'<\1\2>', text)
114115
for a_href in _A_HREF.finditer(text):
115116
href = a_href.group(1)
117+
118+
parsed = urlparse(href)
119+
if parsed.scheme != "" and parsed.scheme not in ["http", "https"]:
120+
continue # Do not unescape links containing disallowed URLs
121+
116122
target = a_href.group(2)
117123

118124
if target:

tests/unit/cmk/gui/test_htmllib_Escaper.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,14 @@ def test_unescape_attribute(inp, out):
7272
"<a href=\"xyz\">abc</a>&lt;script&gt;alert(1)&lt;/script&gt;<a href=\"xyz\">abc</a>",
7373
),
7474
("&nbsp;", None),
75-
# At the moment also javascript URLs are accepted. This will be refused in the next step
76-
("<a href=\"javascript:alert(1)\">abc</a>", None),
75+
# Only http/https are allowed as schemes
76+
("<a href=\"http://checkmk.com/\">abc</a>", None),
77+
("<a href=\"https://checkmk.com/\">abc</a>", None),
78+
("<a href=\"HTTP://CHECKMK.COM/\">abc</a>", None),
79+
("<a href=\"ftp://checkmk.com/\">abc</a>",
80+
"&lt;a href=&quot;ftp://checkmk.com/&quot;&gt;abc</a>"),
81+
("<a href=\"javascript:alert(1)\">abc</a>",
82+
"&lt;a href=&quot;javascript:alert(1)&quot;&gt;abc</a>"),
7783
])
7884
def test_escape_text(inp, out):
7985
if out is None:

0 commit comments

Comments
 (0)