#22675 Validate RSS feed entry link schemes to prevent javascript: XSS

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)
 

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('<b>ok</b>', 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'},
-            ],
-        })
-
-        result = widget.get_feed()
-
+        rss = (
+            b'<?xml version="1.0"?>'
+            b'<rss version="2.0"><channel><title>t</title>'
+            b'<link>http://example.com</link><description>d</description>'
+            b'<item><title>evil</title><link>javascript:alert(1)</link>'
+            b'<description>d</description></item>'
+            b'</channel></rss>'
+        )
+        mock_response = MagicMock()
+        mock_response.content = rss
+
+        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):