Просмотр исходного кода

Closes #22716: Restrict image sources to HTTP(S) and relative URLs

Mohamed Hossam 3 дней назад
Родитель
Сommit
560d595ac3

+ 3 - 0
docs/configuration/security.md

@@ -8,6 +8,9 @@ Default: `('file', 'ftp', 'ftps', 'http', 'https', 'irc', 'mailto', 'sftp', 'ssh
 
 A list of permitted URL schemes referenced when rendering links within NetBox. Note that only the schemes specified in this list will be accepted: If adding your own, be sure to replicate all the default values as well (excluding those schemes which are not desirable).
 
+!!! note
+    Image sources (`<img src="...">`) are limited to HTTP(S) and relative URLs, subject to `ALLOWED_URL_SCHEMES`.
+
 ---
 
 ## AUTH_PASSWORD_VALIDATORS

+ 3 - 0
netbox/utilities/constants.py

@@ -110,6 +110,9 @@ HTML_ALLOWED_ATTRIBUTES = {
     "th": {"align"},
 }
 
+# Allowed URL schemes for image sources (img[src]); applied in addition to ALLOWED_URL_SCHEMES
+IMAGE_URL_SCHEMES = {'http', 'https'}
+
 HTTP_PROXY_SUPPORTED_SOCK_SCHEMAS = ['socks4', 'socks4a', 'socks4h', 'socks5', 'socks5a', 'socks5h']
 HTTP_PROXY_SOCK_RDNS_SCHEMAS = ['socks4h', 'socks4a', 'socks5h', 'socks5a']
 HTTP_PROXY_SUPPORTED_SCHEMAS = ['http', 'https', 'socks4', 'socks4a', 'socks4h', 'socks5', 'socks5a', 'socks5h']

+ 21 - 2
netbox/utilities/html.py

@@ -3,7 +3,7 @@ import re
 import nh3
 from django.utils.html import escape
 
-from .constants import HTML_ALLOWED_ATTRIBUTES, HTML_ALLOWED_TAGS
+from .constants import HTML_ALLOWED_ATTRIBUTES, HTML_ALLOWED_TAGS, IMAGE_URL_SCHEMES
 
 __all__ = (
     'clean_html',
@@ -11,17 +11,36 @@ __all__ = (
     'highlight',
 )
 
+SCHEME_RE = re.compile(r'^([a-zA-Z][a-zA-Z0-9+.-]*):')
+
+# Per the URL spec, browsers ignore leading/trailing C0 control characters & space, and strip any tab or
+# newline characters appearing within a URL. We must normalize accordingly before checking the scheme.
+URL_STRIP_CHARS = ''.join(chr(c) for c in range(0x21))
+URL_REMOVE_CHARS = str.maketrans('', '', '\t\r\n')
+
+
+def _attribute_filter(tag, attr, value):
+    """Returns str to keep/modify attribute, None to remove it."""
+    if tag == 'img' and attr == 'src':
+        match = SCHEME_RE.match(value.strip(URL_STRIP_CHARS).translate(URL_REMOVE_CHARS))
+        if match and match.group(1).lower() not in IMAGE_URL_SCHEMES:
+            return None
+    return value
+
 
 def clean_html(html, schemes):
     """
     Sanitizes HTML based on a whitelist of allowed tags and attributes.
     Also takes a list of allowed URI schemes.
     """
+    url_schemes = set(schemes)
+    attribute_filter = None if url_schemes <= IMAGE_URL_SCHEMES else _attribute_filter
     return nh3.clean(
         html,
         tags=HTML_ALLOWED_TAGS,
         attributes=HTML_ALLOWED_ATTRIBUTES,
-        url_schemes=set(schemes)
+        url_schemes=url_schemes,
+        attribute_filter=attribute_filter,
     )
 
 

+ 101 - 0
netbox/utilities/tests/test_html.py

@@ -0,0 +1,101 @@
+from django.test import SimpleTestCase, tag
+
+from utilities.html import clean_html
+
+TEST_SCHEMES = ['file', 'ftp', 'ssh', 'http', 'https']
+
+
+class CleanHTMLURLPolicyTestCase(SimpleTestCase):
+
+    @tag('regression')
+    def test_img_src_disallowed_schemes(self):
+        """
+        file:/ftp:/ssh: image sources are stripped while non-src attributes survive.
+        (Core regression: these schemes are in ALLOWED_URL_SCHEMES but forbidden for images.)
+        """
+        html = (
+            '<img src="file:///etc" alt="a">'
+            '<img src="ftp://host/i.png" alt="b">'
+            '<img src="ssh://host/i.png" alt="c">'
+        )
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertNotIn('file:///', result)
+        self.assertNotIn('ftp://', result)
+        self.assertNotIn('ssh://', result)
+        self.assertIn('alt="a"', result)
+        self.assertIn('alt="b"', result)
+        self.assertIn('alt="c"', result)
+
+    @tag('regression')
+    def test_img_src_leading_whitespace(self):
+        """Disallowed schemes hidden behind leading whitespace are still stripped."""
+        html = '<img src=" ssh://host/i.png">'
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertNotIn('ssh://', result)
+
+    @tag('regression')
+    def test_img_src_embedded_control_characters(self):
+        """Disallowed schemes obscured by tab/newline/control characters are still stripped."""
+        html = (
+            '<img src="ss&#9;h://host/i.png">'
+            '<img src="fi&#13;le:///etc">'
+            '<img src="&#1;ftp://host/i.png">'
+        )
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertNotIn('h://host', result)
+        self.assertNotIn('le:///etc', result)
+        self.assertNotIn('ftp://host', result)
+
+    @tag('regression')
+    def test_img_src_http_https_and_relative(self):
+        """http/https/relative image sources are retained."""
+        html = (
+            '<img src="https://example.com/i1.png">'
+            '<img src="http://example.com/i2.png">'
+            '<img src="/rel.png">'
+            '<img src="rel.png">'
+            '<img src="//cdn.example.com/i3.png">'
+        )
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertIn('example.com/i1.png', result)
+        self.assertIn('example.com/i2.png', result)
+        self.assertIn('src="/rel.png"', result)
+        self.assertIn('src="rel.png"', result)
+        self.assertIn('src="//cdn.example.com/i3.png"', result)
+
+    @tag('regression')
+    def test_link_href_preserves_all_schemes(self):
+        """a[href] behavior is unchanged — all configured schemes remain allowed."""
+        html = (
+            '<a href="ssh://host">s</a>'
+            '<a href="file:///etc">f</a>'
+            '<a href="ftp://host">f</a>'
+            '<a href="https://example.com">h</a>'
+        )
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertIn('href="ssh://host"', result)
+        self.assertIn('href="file:///etc"', result)
+        self.assertIn('href="ftp://host"', result)
+        self.assertIn('href="https://example.com"', result)
+
+    @tag('regression')
+    def test_mixed_content(self):
+        """Different policies for img vs a in the same input."""
+        html = '<a href="ssh://host">L</a><img src="ssh://host/i.png">'
+        result = clean_html(html, TEST_SCHEMES)
+        self.assertIn('href="ssh://host"', result)
+        self.assertNotIn('ssh://host/i.png', result)
+
+    @tag('regression')
+    def test_javascript_url_blocked(self):
+        """nh3's url_schemes filtering still applies with the attribute filter in place."""
+        html = '<a href="javascript:alert(1)">x</a><img src="javascript:alert(1)">'
+        result = clean_html(html, ['https', 'mailto'])
+        self.assertNotIn('javascript:', result)
+
+    @tag('regression')
+    def test_img_src_respects_allowed_url_schemes(self):
+        """Image sources must respect ALLOWED_URL_SCHEMES, not just IMAGE_URL_SCHEMES."""
+        html = '<img src="http://example.com/i.png">'
+        result = clean_html(html, ['https', 'mailto'])
+        self.assertNotIn('http://example.com', result)