diff --git a/netbox/extras/dashboard/widgets.py b/netbox/extras/dashboard/widgets.py index bb3eac01e..94ca3012b 100644 --- a/netbox/extras/dashboard/widgets.py +++ b/netbox/extras/dashboard/widgets.py @@ -2,7 +2,7 @@ import logging import uuid from functools import cached_property from hashlib import sha256 -from urllib.parse import urlencode +from urllib.parse import urlencode, urlparse import feedparser import requests @@ -16,6 +16,8 @@ from django.utils.translation import gettext as _ from core.models import ObjectType from extras.choices import BookmarkOrderingChoices +from netbox.config import get_config +from utilities.html import clean_html from utilities.object_types import object_type_identifier, object_type_name from utilities.permissions import get_permission_for_model from utilities.proxy import resolve_proxies @@ -356,7 +358,9 @@ class RSSFeedWidget(DashboardWidget): def cache_key(self): url = self.config['feed_url'] url_checksum = sha256(url.encode('utf-8')).hexdigest() - return f'dashboard_rss_{url_checksum}' + # The version segment invalidates entries cached by a pre-sanitization release: such + # entries live under the old key and are never read, so they can't be served unsanitized. + return f'dashboard_rss_2_{url_checksum}' def get_feed(self): if self.config.get('requires_internet') and settings.ISOLATED_DEPLOYMENT: @@ -364,7 +368,8 @@ class RSSFeedWidget(DashboardWidget): 'isolated_deployment': True, } - # Fetch RSS content from cache if available + # Fetch RSS content from cache if available. Cached content is always sanitized before + # it is written (see below), so no sanitization is needed on read. if feed_content := cache.get(self.cache_key): return { 'feed': feedparser.FeedParserDict(feed_content), @@ -390,6 +395,8 @@ class RSSFeedWidget(DashboardWidget): # Cap number of entries max_entries = self.config.get('max_entries') feed['entries'] = feed['entries'][:max_entries] + # Sanitize feed-controlled content before caching/rendering + self.sanitize_entries(feed['entries']) # Cache the feed content cache.set(self.cache_key, dict(feed), self.config.get('cache_timeout')) @@ -397,6 +404,27 @@ class RSSFeedWidget(DashboardWidget): 'feed': feed, } + @staticmethod + def sanitize_entries(entries): + """ + Sanitize feed-controlled entry content in place. The feed URL is untrusted external + content, so we must guard against dangerous URL schemes (e.g. javascript:) in entry + links and sanitize entry summaries as defense-in-depth. + """ + allowed_schemes = get_config().ALLOWED_URL_SCHEMES + for entry in entries: + # Blank any link whose scheme isn't permitted (blocks javascript:, data:, etc.). + # This is the load-bearing control: the template renders entry.link into an href. + if link := entry.get('link'): + result = urlparse(link) + if result.scheme and result.scheme.lower() not in allowed_schemes: + entry['link'] = '' + # Sanitize the summary HTML as defense-in-depth. The template renders entry.summary + # with auto-escaping (not |safe), so this is not currently load-bearing; it guards + # against a future change that renders the summary as markup. + if summary := entry.get('summary'): + entry['summary'] = clean_html(summary, allowed_schemes) + @register_widget class BookmarksWidget(DashboardWidget): diff --git a/netbox/extras/tests/test_dashboard.py b/netbox/extras/tests/test_dashboard.py index 529ab5f9e..bb7498e91 100644 --- a/netbox/extras/tests/test_dashboard.py +++ b/netbox/extras/tests/test_dashboard.py @@ -1,6 +1,9 @@ +from unittest.mock import MagicMock, patch + +from django.core.cache import cache from django.test import RequestFactory, TestCase, tag -from extras.dashboard.widgets import ObjectListWidget +from extras.dashboard.widgets import ObjectListWidget, RSSFeedWidget from extras.templatetags.dashboard import render_widget @@ -49,6 +52,81 @@ class ObjectListWidgetTestCase(TestCase): self.assertTrue('Unable to load content. Could not resolve list URL for:' in rendered) +class RSSFeedWidgetSanitizationTestCase(TestCase): + """ + Feed entry content is externally controlled and untrusted. Links must be validated against + ALLOWED_URL_SCHEMES so dangerous schemes (e.g. javascript:) cannot become clickable XSS sinks. + """ + + @tag('regression') + def test_sanitize_entries_blanks_disallowed_schemes(self): + entries = [ + {'link': 'javascript:alert(document.cookie)', 'title': 't1'}, + {'link': 'JavaScript:alert(1)', 'title': 't2'}, # case-insensitive + {'link': 'data:text/html,', 'title': 't3'}, + {'link': 'vbscript:msgbox(1)', 'title': 't4'}, + ] + RSSFeedWidget.sanitize_entries(entries) + for entry in entries: + self.assertEqual(entry['link'], '', msg=f"Failed to blank {entry['title']}") + + @tag('regression') + def test_sanitize_entries_preserves_allowed_links(self): + entries = [ + {'link': 'https://example.com/post', 'title': 't1'}, + {'link': 'http://example.com/post', 'title': 't2'}, + {'link': 'mailto:user@example.com', 'title': 't3'}, + {'link': '/relative/path', 'title': 't4'}, # schemeless relative link + ] + expected = [e['link'] for e in entries] + RSSFeedWidget.sanitize_entries(entries) + self.assertEqual([e['link'] for e in entries], expected) + + @tag('regression') + def test_sanitize_entries_cleans_summary_html(self): + entries = [ + {'link': 'https://example.com', 'title': 't1', 'summary': 'ok'}, + ] + RSSFeedWidget.sanitize_entries(entries) + self.assertNotIn('