diff --git a/netbox/extras/dashboard/widgets.py b/netbox/extras/dashboard/widgets.py index 03f85a7f0..94ca3012b 100644 --- a/netbox/extras/dashboard/widgets.py +++ b/netbox/extras/dashboard/widgets.py @@ -358,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: @@ -366,14 +368,11 @@ 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): - feed = feedparser.FeedParserDict(feed_content) - # Sanitize on read as well: cached content may have been written by a pre-fix - # release (during an upgrade) and thus not yet sanitized. Idempotent. - self.sanitize_entries(feed.get('entries', [])) return { - 'feed': feed, + 'feed': feedparser.FeedParserDict(feed_content), } # Fetch feed content from remote server @@ -414,12 +413,15 @@ class RSSFeedWidget(DashboardWidget): """ allowed_schemes = get_config().ALLOWED_URL_SCHEMES for entry in entries: - # Blank any link whose scheme isn't permitted (blocks javascript:, data:, etc.) + # 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 + # 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) diff --git a/netbox/extras/tests/test_dashboard.py b/netbox/extras/tests/test_dashboard.py index 43e8e9562..bb7498e91 100644 --- a/netbox/extras/tests/test_dashboard.py +++ b/netbox/extras/tests/test_dashboard.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock, patch + from django.core.cache import cache from django.test import RequestFactory, TestCase, tag @@ -90,26 +92,39 @@ class RSSFeedWidgetSanitizationTestCase(TestCase): self.assertIn('ok', entries[0]['summary']) @tag('regression') - def test_get_feed_sanitizes_cached_content(self): + def test_get_feed_sanitizes_before_caching(self): """ - Content cached by a pre-fix release must be sanitized on read, not served verbatim. + Fetched feed content must be sanitized before it is rendered or written to the cache, + so a poisoned link is never stored or served. """ widget = RSSFeedWidget(config={ 'feed_url': 'https://example.com/feed.xml', 'requires_internet': False, 'max_entries': 10, + 'cache_timeout': 3600, }) - # Simulate a poisoned feed left in the cache by an older release - cache.set(widget.cache_key, { - 'bozo': False, - 'entries': [ - {'link': 'javascript:alert(1)', 'title': 'evil', 'summary': 'x'}, - ], - }) + rss = ( + b'' + b't' + b'http://example.comd' + b'eviljavascript:alert(1)' + b'd' + b'' + ) + mock_response = MagicMock() + mock_response.content = rss - result = widget.get_feed() + with ( + patch('extras.dashboard.widgets.requests.get', return_value=mock_response), + patch('extras.dashboard.widgets.resolve_proxies', return_value={}), + ): + result = widget.get_feed() + # The rendered feed is sanitized... self.assertEqual(result['feed']['entries'][0]['link'], '') + # ...and the cached copy is sanitized too (never stored poisoned). + cached = cache.get(widget.cache_key) + self.assertEqual(cached['entries'][0]['link'], '') class RenderWidgetTemplateTagTestCase(TestCase):