🐛 Fixed unsafe notification HTML

ghost/core/core/server/services/notifications/notification-email.ts

@@ -1,4 +1,4 @@
-import {sanitizeEmailHtml} from './sanitize-email-html';
+import {sanitizeNotificationHtml} from './sanitize-notification-html';
 
 interface Mailer {
     send(options: {to: string; subject: string; html: string; text?: string}): Promise<unknown>;
@@ -48,7 +48,7 @@ export class NotificationEmailService {
         if (!to.length) {
             return;
         }
-        const message = sanitizeEmailHtml(content);
+        const message = sanitizeNotificationHtml(content);
         const siteUrl = this.getSiteUrl();
         for (const recipient of to) {
             const {html, text} = await this.generateEmailContent({

ghost/core/core/server/services/notifications/notifications.js

@@ -5,10 +5,20 @@ const errors = require('@tryghost/errors');
 const ghostVersion = require('@tryghost/version');
 const tpl = require('@tryghost/tpl');
 const ObjectId = require('bson-objectid').default;
+const {sanitizeNotificationHtml} = require('./sanitize-notification-html');
 
 const messages = {
     noPermissionToDismissNotif: 'You do not have permission to dismiss this notification.',
-    notificationDoesNotExist: 'Notification does not exist.'
+    notificationDoesNotExist: 'Notification does not exist.',
+    invalidNotificationMessage: 'Notification message must be a string.'
+};
+
+const sanitizeMessage = (message) => {
+    if (typeof message !== 'string') {
+        return '';
+    }
+
+    return sanitizeNotificationHtml(message);
 };
 
 class Notifications {
@@ -42,6 +52,9 @@ class Notifications {
 
         allNotifications.forEach((notification) => {
             notification.addedAt = moment(notification.addedAt).toDate();
+            // Sanitising on read protects installs with notifications that were
+            // persisted before write-side sanitisation was introduced.
+            notification.message = sanitizeMessage(notification.message);
         });
 
         return allNotifications;
@@ -145,7 +158,15 @@ class Notifications {
             });
 
             if (!isDuplicate) {
-                notificationsToAdd.push(Object.assign({}, defaults, notification, overrides));
+                if (typeof notification.message !== 'string') {
+                    throw new errors.ValidationError({
+                        message: tpl(messages.invalidNotificationMessage)
+                    });
+                }
+
+                const notificationToAdd = Object.assign({}, defaults, notification, overrides);
+                notificationToAdd.message = sanitizeMessage(notificationToAdd.message);
+                notificationsToAdd.push(notificationToAdd);
             }
         });
 

ghost/core/core/server/services/notifications/sanitize-email-html.ts → ghost/core/core/server/services/notifications/sanitize-notification-html.ts

@@ -1,9 +1,8 @@
 import sanitizeHtml from 'sanitize-html';
 
-// Even though the upstream feed is operated by Ghost org, the resulting HTML
-// is rendered into admin inboxes on the receiving install. A compromised or
-// malformed feed entry must not be able to ship scripts, event handlers, or
-// non-http(s) URLs to recipients via this path.
+// Notification bodies are rendered as HTML in Ghost Admin and notification
+// emails. Keep the content semantic and exclude executable markup, event
+// handlers, unsafe URLs, images and inline styles.
 const ALLOWED_TAGS = [
     'p', 'br', 'hr',
     'strong', 'b', 'em', 'i', 'u', 'code',
@@ -30,6 +29,6 @@ const SANITIZE_OPTIONS: sanitizeHtml.IOptions = {
     }
 };
 
-export function sanitizeEmailHtml(html: string): string {
+export function sanitizeNotificationHtml(html: string): string {
     return sanitizeHtml(html, SANITIZE_OPTIONS);
 }

ghost/core/test/unit/server/services/notifications/notifications.test.js

@@ -47,9 +47,68 @@ describe('Notifications Service', function () {
             assert.equal(createdNotification.message, 'Hello test world!');
             assert.equal(createdNotification.createdAtVersion, '4.1.0');
         });
+
+        it('sanitizes notification HTML before returning it for storage', function () {
+            const settingsCache = {
+                get: sinon.fake.returns([])
+            };
+
+            const notificationsSvc = new Notifications({settingsCache});
+            const {notificationsToAdd} = notificationsSvc.add({
+                notifications: [{
+                    message: '<p>Keep <strong>formatting</strong> and <a href="https://ghost.org" onclick="alert(1)">safe links</a>.</p><script>alert(1)</script>'
+                }]
+            });
+
+            assert.equal(notificationsToAdd[0].message, '<p>Keep <strong>formatting</strong> and <a href="https://ghost.org" target="_blank" rel="noopener noreferrer">safe links</a>.</p>');
+        });
+
+        it('rejects non-string messages', function () {
+            const settingsCache = {
+                get: sinon.fake.returns([])
+            };
+
+            const notificationsSvc = new Notifications({settingsCache});
+
+            assert.throws(() => notificationsSvc.add({
+                notifications: [{message: {invalid: true}}]
+            }), {
+                message: 'Notification message must be a string.'
+            });
+        });
     });
 
     describe('browse', function () {
+        it('sanitizes notifications that were stored before write sanitization', function () {
+            const settingsCache = {
+                get: sinon.fake.returns([{
+                    custom: true,
+                    message: '<p>Keep <em>formatting</em>.</p><img src=x onerror="alert(1)"><script>alert(1)</script>',
+                    addedAt: '2021-03-17T01:41:20.906Z'
+                }])
+            };
+
+            const notificationSvc = new Notifications({settingsCache});
+            const notifications = notificationSvc.browse({user: owner});
+
+            assert.equal(notifications[0].message, '<p>Keep <em>formatting</em>.</p>');
+        });
+
+        it('neutralizes non-string messages that were stored previously', function () {
+            const settingsCache = {
+                get: sinon.fake.returns([{
+                    custom: true,
+                    message: {invalid: true},
+                    addedAt: '2021-03-17T01:41:20.906Z'
+                }])
+            };
+
+            const notificationSvc = new Notifications({settingsCache});
+            const notifications = notificationSvc.browse({user: owner});
+
+            assert.equal(notifications[0].message, '');
+        });
+
         it('can browse non-major version upgrade notifications', function () {
             const settingsCache = {
                 get: sinon.fake.returns([{

ghost/core/test/unit/server/services/notifications/sanitize-notification-html.test.ts

@@ -0,0 +1,58 @@
+import assert from 'node:assert/strict';
+import {sanitizeNotificationHtml} from '../../../../../core/server/services/notifications/sanitize-notification-html';
+
+const RELEASE_NOTIFICATION = 'Ghost <a href="https://github.com/TryGhost/Ghost/releases">v6.55.0</a> has been released, <a href="https://ghost.org/update/?v=6.50.0">click here</a> to upgrade.';
+
+const CRITICAL_NOTIFICATION = `
+    <div style="text-align:center;">
+        <div style="display:inline-block;max-width:560px;text-align:left;padding:40px 24px;font-family:Helvetica,Arial,sans-serif;color:#3A464C;font-size:16px;line-height:1.6;">
+            <div style="text-align:center;margin:0 0 28px;">
+                <img src="https://static.ghost.org/v4.0.0/images/ghost-orb-2.png" width="48" height="48" alt="Ghost">
+            </div>
+            <h1 style="font-size:21px;color:#15212A;font-weight:600;line-height:1.3;margin:0 0 20px;">Critical Ghost security update</h1>
+            <p style="margin:0 0 16px;">Hi there,</p>
+            <p style="margin:0 0 16px;">A critical security update for Ghost has been released that patches recently reported vulnerabilities. Please update your Ghost install to the latest version as soon as possible, and consider resetting your authentication credentials:</p>
+            <p style="margin:0 0 16px;"><a href="https://ghost.org/help/auth-reset" style="color:#15212A;font-weight:600;">ghost.org/help/auth-reset</a></p>
+            <p style="margin:0 0 16px;">Full security advisories are published here:</p>
+            <p style="margin:0 0 16px;"><a href="https://github.com/TryGhost/Ghost/security/advisories" style="color:#15212A;font-weight:600;">github.com/TryGhost/Ghost/security/advisories</a></p>
+            <p style="margin:0 0 16px;">How to update: <a href="https://docs.ghost.org/install/docker#updating-ghost" style="color:#15212A;font-weight:600;">Docker</a> or <a href="https://docs.ghost.org/update" style="color:#15212A;font-weight:600;">Ghost-CLI</a>.</p>
+            <p style="margin:0;border-top:1px solid #EEF5F8;padding-top:20px;font-size:12px;color:#738A94;">Sent to administrators of Ghost sites.</p>
+        </div>
+    </div>
+`;
+
+describe('sanitizeNotificationHtml', function () {
+    it('preserves release notification links', function () {
+        const output = sanitizeNotificationHtml(RELEASE_NOTIFICATION);
+
+        assert.equal(output, 'Ghost <a href="https://github.com/TryGhost/Ghost/releases" target="_blank" rel="noopener noreferrer">v6.55.0</a> has been released, <a href="https://ghost.org/update/?v=6.50.0" target="_blank" rel="noopener noreferrer">click here</a> to upgrade.');
+    });
+
+    it('preserves the semantic content and links used by critical notifications', function () {
+        const output = sanitizeNotificationHtml(CRITICAL_NOTIFICATION);
+
+        assert.equal(
+            output.replace(/\s+/g, ' ').trim(),
+            '<h1>Critical Ghost security update</h1> <p>Hi there,</p> <p>A critical security update for Ghost has been released that patches recently reported vulnerabilities. Please update your Ghost install to the latest version as soon as possible, and consider resetting your authentication credentials:</p> <p><a href="https://ghost.org/help/auth-reset" target="_blank" rel="noopener noreferrer">ghost.org/help/auth-reset</a></p> <p>Full security advisories are published here:</p> <p><a href="https://github.com/TryGhost/Ghost/security/advisories" target="_blank" rel="noopener noreferrer">github.com/TryGhost/Ghost/security/advisories</a></p> <p>How to update: <a href="https://docs.ghost.org/install/docker#updating-ghost" target="_blank" rel="noopener noreferrer">Docker</a> or <a href="https://docs.ghost.org/update" target="_blank" rel="noopener noreferrer">Ghost-CLI</a>.</p> <p>Sent to administrators of Ghost sites.</p>'
+        );
+    });
+
+    it('removes executable markup, unsafe URLs, images and inline styles', function () {
+        const output = sanitizeNotificationHtml(`
+            <script>alert('nope')</script>
+            <img src="https://example.com/tracker.png" onerror="alert(1)" width="100vw">
+            <a href="javascript:alert(1)" onclick="alert(1)" style="color:#123456">Unsafe link</a>
+        `);
+
+        assert.doesNotMatch(output, /script|onerror|onclick|javascript:|style|tracker\.png|<img/);
+        assert.match(output, /<a target="_blank" rel="noopener noreferrer">Unsafe link<\/a>/);
+    });
+
+    it('is idempotent for malformed namespaced markup', function () {
+        const input = '<svg><foreignObject><p onclick="alert(1)">Keep me</p><a href="javascript:alert(1)">Unsafe link</a></foreignObject></svg><math><mtext><img src=x onerror="alert(1)">Math text</mtext></math>';
+        const output = sanitizeNotificationHtml(input);
+
+        assert.equal(output, '<p>Keep me</p><a target="_blank" rel="noopener noreferrer">Unsafe link</a>Math text');
+        assert.equal(sanitizeNotificationHtml(output), output);
+    });
+});