CVE-2020-28919: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: A stored cross site scripting (XSS) vulnerability in Checkmk 1.6.0x prior to 1.6.0p19 allows an authenticated remote attacker to inject arbitrary JavaScript via a javascript: URL in a view title.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-01-15T17:15Z
Repository: https://github.com/tribe29/checkmk
Language: Python
Files changed: cmk/gui/escaping.py, tests/unit/cmk/gui/test_htmllib_Escaper.py
Commit date: 2020-10-21 09:29:44 +0200
Commit message: 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

Diff: diff --git a/cmk/gui/escaping.py b/cmk/gui/escaping.py
index 03dcdda9ed..6f7db543e4 100644
--- a/cmk/gui/escaping.py
+++ b/cmk/gui/escaping.py
@@ -5,10 +5,11 @@
 # conditions defined in the file COPYING, which is part of this source code package.
 
 from html import escape as html_escape
 import re
 from typing import Union
+from urllib.parse import urlparse
 
 from six import ensure_str
 
 from cmk.gui.utils.html import HTML
 
@@ -111,10 +112,15 @@ def escape_text(text: EscapableEntity) -> str:
 
     text = escape_attribute(text)
     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:
             unescaped_tag = "<a href=\"%s\" target=\"%s\">" % (href, target)
         else:
diff --git a/tests/unit/cmk/gui/test_htmllib_Escaper.py b/tests/unit/cmk/gui/test_htmllib_Escaper.py
index 3a34499bad..6a015c8220 100644
--- a/tests/unit/cmk/gui/test_htmllib_Escaper.py
+++ b/tests/unit/cmk/gui/test_htmllib_Escaper.py
@@ -70,12 +70,18 @@ def test_unescape_attribute(inp, out):
         (
             "<a href=\"xyz\">abc</a><script>alert(1)</script><a href=\"xyz\">abc</a>",
             "<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:
         out = inp
     assert escaping.escape_text(inp) == out

Vulnerable code:         # At the moment also javascript URLs are accepted. This will be refused in the next step
        ("<a href=\"javascript:alert(1)\">abc</a>", None),
Fixed code: from urllib.parse import urlparse

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

        # 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>"),