Skip to content

Commit

Permalink
🔒 Escaped HTML in bookmark fields (#1390)
Browse files Browse the repository at this point in the history
ref https://linear.app/ghost/issue/ENG-1751

- in order to prevent potential cross-site scripting (XSS) vulnerabilities or HTML injection, we are now escaping bookmark fields for email
- bookmark fields were already escaped for web, by using textContent
  • Loading branch information
sagzy authored Nov 12, 2024
1 parent 7dd82ba commit 00542be
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 22 deletions.
30 changes: 11 additions & 19 deletions packages/kg-default-nodes/lib/nodes/bookmark/bookmark-renderer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {addCreateDocumentOption} from '../../utils/add-create-document-option';
import {renderEmptyContainer} from '../../utils/render-empty-container';
import {escapeHtml} from '../../utils/escape-html';

export function renderBookmarkNode(node, options = {}) {
addCreateDocumentOption(options);
Expand All @@ -17,15 +18,6 @@ export function renderBookmarkNode(node, options = {}) {
}
}

function escapeHtml(unsafe) {
return unsafe
.replace(/&/g, '&')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#039;');
}

function truncateText(text, maxLength) {
if (text && text.length > maxLength) {
return text.substring(0, maxLength - 1).trim() + '…';
Expand All @@ -45,7 +37,7 @@ function truncateHtml(text, maxLength, maxLengthMobile) {
if (text.length <= maxLengthMobile) {
return escapeHtml(text);
}

if (text && text.length > maxLengthMobile) {
let ellipsis = '';

Expand All @@ -62,18 +54,18 @@ function truncateHtml(text, maxLength, maxLengthMobile) {
}

function emailTemplate(node, document) {
const title = node.title;
const publisher = node.publisher;
const author = node.author;
const icon = node.icon;
const description = node.description;
const url = node.url;
const thumbnail = node.thumbnail;
const caption = node.caption;
const title = escapeHtml(node.title);
const publisher = escapeHtml(node.publisher);
const author = escapeHtml(node.author);
const icon = escapeHtml(node.icon);
const description = escapeHtml(node.description);
const url = escapeHtml(node.url);
const thumbnail = escapeHtml(node.thumbnail);
const caption = escapeHtml(node.caption);

const element = document.createElement('div');

const html =
const html =
`
<!--[if !mso !vml]-->
<figure class="kg-card kg-bookmark-card ${caption ? `kg-card-hascaption` : ''}">
Expand Down
2 changes: 1 addition & 1 deletion packages/kg-default-nodes/lib/utils/escape-html.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Escape HTML special characters
* @param {string} unsafe
* @param {string} unsafe
* @returns string
*/
export function escapeHtml(unsafe) {
Expand Down
54 changes: 52 additions & 2 deletions packages/kg-default-nodes/test/nodes/bookmark.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describe('BookmarkNode', function () {
describe('urlTransformMap', function () {
it('contains the expected URL mapping', editorTest(function () {
BookmarkNode.urlTransformMap.should.deepEqual({
'url': 'url',
url: 'url',
'metadata.icon': 'url',
'metadata.thumbnail': 'url'
});
Expand Down Expand Up @@ -206,6 +206,56 @@ describe('BookmarkNode', function () {

element.outerHTML.should.equal('<span></span>');
}));

it('escapes HTML for web', editorTest(function () {
dataset = {
url: 'https://www.fake.org/',
metadata: {
icon: 'https://www.fake.org/favicon.ico',
title: 'Ghost: Independent technology <script>alert("XSS")</script> for modern publishing.',
description: 'doing "kewl" stuff',
author: 'fa\'ker',
publisher: 'Fake <script>alert("XSS")</script>',
thumbnail: 'https://fake.org/image.png'
},
caption: 'caption here'
};
const bookmarkNode = $createBookmarkNode(dataset);
const {element} = bookmarkNode.exportDOM(exportOptions);

element.innerHTML.should.containEql('Ghost: Independent technology &lt;script&gt;alert("XSS")&lt;/script&gt; for modern publishing.');
element.innerHTML.should.containEql('doing "kewl" stuff');
element.innerHTML.should.containEql('fa\'ker');
element.innerHTML.should.containEql('Fake &lt;script&gt;alert("XSS")&lt;/script&gt;');
element.innerHTML.should.containEql('https://fake.org/image.png');
}));

it('escapes HTML for email', editorTest(function () {
const options = {
target: 'email'
};
dataset = {
url: 'https://www.fake.org/',
metadata: {
icon: 'https://www.fake.org/favicon.ico',
title: 'Ghost: Independent technology <script>alert("XSS")</script> for modern publishing.',
description: 'doing "kewl" stuff',
author: 'fa\'ker',
publisher: 'Fake <script>alert("XSS")</script>',
thumbnail: 'https://fake.org/image.png'
},
caption: 'caption here'
};
const bookmarkNode = $createBookmarkNode(dataset);
const {element} = bookmarkNode.exportDOM({...exportOptions, ...options});

element.innerHTML.should.containEql('<!--[if !mso !vml]-->'); // Check that email template is used
element.innerHTML.should.containEql('Ghost: Independent technology &lt;script&gt;alert("XSS")&lt;/script&gt; for modern publishing.');
element.innerHTML.should.containEql('doing &amp;quot;kewl&amp;quot; stuff');
element.innerHTML.should.containEql('fa\'ker');
element.innerHTML.should.containEql('Fake &lt;script&gt;alert("XSS")&lt;/script&gt;');
element.innerHTML.should.containEql('https://fake.org/image.png');
}));
});

describe('exportJSON', function () {
Expand Down Expand Up @@ -277,7 +327,7 @@ describe('BookmarkNode', function () {

it('urlTransformMap', editorTest(function () {
BookmarkNode.urlTransformMap.should.deepEqual({
'url': 'url',
url: 'url',
'metadata.icon': 'url',
'metadata.thumbnail': 'url'
});
Expand Down

0 comments on commit 00542be

Please sign in to comment.