Skip to content

Commit c1d2b62

Browse files
committed
🐛 Fixed stored XSS and mangled structured data in JSON-LD output
ref #28957 - tag names, post keywords, and the site title were serialized raw into the inline <script type="application/ld+json"> block, letting an Editor-controlled value like `foo</script><script>...` break out of the script element and run arbitrary JS for anonymous visitors on tag and post pages - escapes the breakout-relevant characters (< > U+2028 U+2029) as JSON \u escapes at the single serialization boundary in ghost_head, so every field is covered at once instead of relying on per-field escaping that is easy to forget - removed the per-field escapeExpression calls from schema.js: HTML-entity escaping is the wrong layer here — JSON-LD consumers (Google et al.) parse the block as JSON and never HTML-decode, so it silently corrupted structured data (e.g. `Tom & Jerry` was indexed as `Tom &amp; Jerry`). JSON \u escapes are both safe and lossless, so legitimate `& ' "` now round-trip correctly - added a regression test proving breakout is neutralised while data round-trips, and updated the snapshot/assertions that were capturing the old corruption
1 parent 4f5b449 commit c1d2b62

5 files changed

Lines changed: 103 additions & 35 deletions

File tree

ghost/core/core/frontend/helpers/ghost_head.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,34 @@ const {getFrontendAppConfig, getDataAttributes} = require('../utils/frontend-app
2222

2323
const {get: getMetaData, getAssetUrl} = metaData;
2424

25+
/**
26+
* Escape a serialized JSON string for safe inclusion inside an inline
27+
* `<script type="application/ld+json">` block.
28+
*
29+
* A `<script>` element is HTML *raw text*: the parser stops only at the literal
30+
* substring `</script`, and it never decodes HTML character references. So
31+
* HTML-entity escaping (e.g. `&lt;`) is the wrong tool here — worse than
32+
* unnecessary, it corrupts the data, because JSON-LD consumers (Google's
33+
* structured-data parser and friends) read the block as JSON and never
34+
* HTML-decode it, so `Tom & Jerry` would be indexed as the literal
35+
* `Tom &amp; Jerry`.
36+
*
37+
* Instead we escape only the breakout-relevant characters as JSON `\u` escapes.
38+
* `JSON.parse` — and every conformant structured-data parser — decodes them
39+
* back to the original character, so the data round-trips exactly while
40+
* `</script>` / `<!--` sequences can no longer form (both begin with `<`).
41+
*
42+
* @param {string} json - the output of `JSON.stringify`
43+
* @returns {string}
44+
*/
45+
function escapeJsonLd(json) {
46+
return json
47+
.replace(/</g, '\\u003c')
48+
.replace(/>/g, '\\u003e')
49+
.replace(/\u2028/g, '\\u2028')
50+
.replace(/\u2029/g, '\\u2029');
51+
}
52+
2553
function writeMetaTag(property, content, type) {
2654
type = type || property.substring(0, 7) === 'twitter' ? 'name' : 'property';
2755
return '<meta ' + type + '="' + property + '" content="' + content + '">';
@@ -327,7 +355,7 @@ module.exports = async function ghost_head(options) { // eslint-disable-line cam
327355

328356
if (!excludeList.has('schema') && meta.schema) {
329357
head.push('<script type="application/ld+json">\n' +
330-
JSON.stringify(meta.schema, null, ' ') +
358+
escapeJsonLd(JSON.stringify(meta.schema, null, ' ')) +
331359
'\n </script>\n');
332360
}
333361
}
@@ -440,3 +468,6 @@ module.exports = async function ghost_head(options) { // eslint-disable-line cam
440468
};
441469

442470
module.exports.async = true;
471+
472+
// Exported for unit testing.
473+
module.exports.escapeJsonLd = escapeJsonLd;

ghost/core/core/frontend/meta/schema.js

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
const config = require('../../shared/config');
2-
const escapeExpression = require('../services/theme-engine/engine').escapeExpression;
32
const socialUrls = require('@tryghost/social-urls');
43
const _ = require('lodash');
54

5+
// NOTE: values here are intentionally NOT HTML-escaped. This object is serialized
6+
// with JSON.stringify into an inline <script type="application/ld+json"> block by
7+
// the ghost_head helper, which escapes the breakout-relevant characters as JSON
8+
// \u escapes (see escapeJsonLd there). HTML-entity escaping here would be both the
9+
// wrong layer and actively harmful — JSON-LD consumers (Google et al.) parse the
10+
// block as JSON and never HTML-decode, so `Tom & Jerry` would be indexed as the
11+
// literal `Tom &amp; Jerry`.
12+
613
function schemaImageObject(metaDataVal) {
714
let imageObject;
815
if (!metaDataVal || !metaDataVal.url) {
@@ -27,7 +34,7 @@ function schemaPublisherObject(metaDataVal) {
2734

2835
publisherObject = {
2936
'@type': 'Organization',
30-
name: escapeExpression(metaDataVal.site.title),
37+
name: metaDataVal.site.title,
3138
url: metaDataVal.site.url || null,
3239
logo: schemaImageObject(metaDataVal.site.logo) || null
3340
};
@@ -66,12 +73,12 @@ function trimSameAs(author) {
6673
const sameAs = [];
6774

6875
if (author.website) {
69-
sameAs.push(escapeExpression(author.website));
76+
sameAs.push(author.website);
7077
}
7178

7279
SOCIAL_PLATFORMS.forEach((platform) => {
7380
if (author[platform] && typeof socialUrls[platform] === 'function') {
74-
sameAs.push(escapeExpression(socialUrls[platform](author[platform])));
81+
sameAs.push(socialUrls[platform](author[platform]));
7582
}
7683
});
7784

@@ -86,20 +93,18 @@ function trimSameAs(author) {
8693
function buildContributorObjects(authors) {
8794
return authors.map(author => trimSchema({
8895
'@type': 'Person',
89-
name: escapeExpression(author.name),
96+
name: author.name,
9097
image: author.profile_image ? schemaImageObject({url: author.profile_image}) : null,
9198
url: author.url || null,
9299
sameAs: trimSameAs(author),
93-
description: author.meta_description ?
94-
escapeExpression(author.meta_description) :
95-
null
100+
description: author.meta_description || null
96101
}));
97102
}
98103

99104
function getPostSchema(metaData, data) {
100105
// CASE: metaData.excerpt for post context is populated by either the custom excerpt, the meta description,
101106
// or the automated excerpt of 50 words. It is empty for any other context.
102-
const description = metaData.excerpt ? escapeExpression(metaData.excerpt) : null;
107+
const description = metaData.excerpt || null;
103108

104109
let schema;
105110

@@ -111,16 +116,14 @@ function getPostSchema(metaData, data) {
111116
publisher: schemaPublisherObject(metaData),
112117
author: {
113118
'@type': 'Person',
114-
name: escapeExpression(data[context].primary_author.name),
119+
name: data[context].primary_author.name,
115120
image: schemaImageObject(metaData.authorImage),
116121
url: metaData.authorUrl,
117122
sameAs: trimSameAs(data[context].primary_author),
118-
description: data[context].primary_author.metaDescription ?
119-
escapeExpression(data[context].primary_author.metaDescription) :
120-
null
123+
description: data[context].primary_author.metaDescription || null
121124
},
122125
contributor: data[context].authors && data[context].authors.length > 1 ? buildContributorObjects(data[context].authors.slice(1)) : null,
123-
headline: escapeExpression(metaData.metaTitle),
126+
headline: metaData.metaTitle,
124127
url: metaData.url,
125128
datePublished: metaData.publishedDate,
126129
dateModified: metaData.modifiedDate,
@@ -143,9 +146,7 @@ function getHomeSchema(metaData) {
143146
name: metaData.site.title,
144147
image: schemaImageObject(metaData.coverImage),
145148
mainEntityOfPage: metaData.url,
146-
description: metaData.metaDescription ?
147-
escapeExpression(metaData.metaDescription) :
148-
null
149+
description: metaData.metaDescription || null
149150
};
150151
return trimSchema(schema);
151152
}
@@ -159,9 +160,7 @@ function getTagSchema(metaData, data) {
159160
image: schemaImageObject(metaData.coverImage),
160161
name: data.tag.name,
161162
mainEntityOfPage: metaData.url,
162-
description: metaData.metaDescription ?
163-
escapeExpression(metaData.metaDescription) :
164-
null
163+
description: metaData.metaDescription || null
165164
};
166165

167166
return trimSchema(schema);
@@ -172,13 +171,11 @@ function getAuthorSchema(metaData, data) {
172171
'@context': 'https://schema.org',
173172
'@type': 'Person',
174173
sameAs: trimSameAs(data.author),
175-
name: escapeExpression(data.author.name),
174+
name: data.author.name,
176175
url: metaData.authorUrl,
177176
image: schemaImageObject(metaData.authorImage) || schemaImageObject(metaData.coverImage),
178177
mainEntityOfPage: metaData.authorUrl,
179-
description: metaData.metaDescription ?
180-
escapeExpression(metaData.metaDescription) :
181-
null
178+
description: metaData.metaDescription || null
182179
};
183180

184181
return trimSchema(schema);

ghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4230,7 +4230,7 @@ Object {
42304230
\\"https://x.com/testuser\\"
42314231
]
42324232
},
4233-
\\"headline\\": \\"Welcome to Ghost &quot;test&quot;\\",
4233+
\\"headline\\": \\"Welcome to Ghost \\\\\\"test\\\\\\"\\",
42344234
\\"url\\": \\"http://localhost:65530/post/\\",
42354235
\\"datePublished\\": \\"1970-01-01T00:00:00.000Z\\",
42364236
\\"dateModified\\": \\"1970-01-01T00:00:00.000Z\\",
@@ -4239,7 +4239,7 @@ Object {
42394239
\\"url\\": \\"http://localhost:65530/content/images/test-image.png\\"
42404240
},
42414241
\\"keywords\\": \\"tag1, tag2, tag3\\",
4242-
\\"description\\": \\"site &quot;test&quot; description\\",
4242+
\\"description\\": \\"site \\\\\\"test\\\\\\" description\\",
42434243
\\"mainEntityOfPage\\": \\"http://localhost:65530/post/\\"
42444244
}
42454245
</script>
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
const assert = require('node:assert/strict');
2+
3+
const {escapeJsonLd} = require('../../../../core/frontend/helpers/ghost_head');
4+
5+
describe('ghost_head escapeJsonLd', function () {
6+
it('neutralises a </script> breakout without losing the data', function () {
7+
const payload = 'foo</script><script>alert(document.domain)</script>';
8+
const escaped = escapeJsonLd(JSON.stringify({name: payload}));
9+
10+
// No literal </script> can survive to close the inline JSON-LD element.
11+
assert.ok(!escaped.includes('</script>'), 'must not contain a literal </script>');
12+
assert.ok(escaped.includes('\\u003c/script\\u003e'), 'the < must be JSON-unicode-escaped');
13+
14+
// ...but a JSON-LD consumer (Google et al.) decodes it back to the original.
15+
assert.equal(JSON.parse(escaped).name, payload);
16+
});
17+
18+
it('escapes <!-- comment breakout as well', function () {
19+
const escaped = escapeJsonLd(JSON.stringify({name: '<!--'}));
20+
21+
assert.ok(!escaped.includes('<!--'));
22+
assert.equal(JSON.parse(escaped).name, '<!--');
23+
});
24+
25+
it('leaves SEO-significant characters intact (no HTML-entity corruption)', function () {
26+
// These characters are safe inside <script> raw text and must round-trip
27+
// verbatim — HTML-entity escaping them (&amp;, &#x27;, &quot;) would be
28+
// indexed literally by structured-data parsers, which never HTML-decode.
29+
const value = 'Tom & Jerry\'s "Q&A" > answers';
30+
const escaped = escapeJsonLd(JSON.stringify({keywords: value}));
31+
32+
assert.ok(!escaped.includes('&amp;'), 'ampersand must not be HTML-escaped');
33+
assert.ok(!escaped.includes('&#x27;'), 'apostrophe must not be HTML-escaped');
34+
assert.ok(!escaped.includes('&quot;'), 'quote must not be HTML-escaped');
35+
assert.equal(JSON.parse(escaped).keywords, value);
36+
});
37+
});

ghost/core/test/unit/frontend/meta/schema.test.js

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ describe('getSchema', function () {
514514
mainEntityOfPage: 'http://mysite.com/author/me/',
515515
name: 'Author Name',
516516
sameAs: [
517-
'http://myblogsite.com/?user&#x3D;bambedibu&amp;a&#x3D;&lt;script&gt;alert(&quot;bambedibu&quot;)&lt;/script&gt;',
517+
'http://myblogsite.com/?user=bambedibu&a=<script>alert("bambedibu")</script>',
518518
'https://x.com/testuser'
519519
],
520520
url: 'http://mysite.com/author/me/'
@@ -569,7 +569,7 @@ describe('getSchema', function () {
569569
mainEntityOfPage: 'http://mysite.com/author/me/',
570570
name: 'Author Name',
571571
sameAs: [
572-
'http://myblogsite.com/?user&#x3D;bambedibu&amp;a&#x3D;&lt;script&gt;alert(&quot;bambedibu&quot;)&lt;/script&gt;',
572+
'http://myblogsite.com/?user=bambedibu&a=<script>alert("bambedibu")</script>',
573573
'https://x.com/testuser'
574574
],
575575
url: 'http://mysite.com/author/me/'
@@ -617,7 +617,7 @@ describe('getSchema', function () {
617617
mainEntityOfPage: 'http://mysite.com/author/me/',
618618
name: 'Author Name',
619619
sameAs: [
620-
'http://myblogsite.com/?user&#x3D;bambedibu&amp;a&#x3D;&lt;script&gt;alert(&quot;bambedibu&quot;)&lt;/script&gt;',
620+
'http://myblogsite.com/?user=bambedibu&a=<script>alert("bambedibu")</script>',
621621
'https://x.com/testuser'
622622
],
623623
url: 'http://mysite.com/author/me/'
@@ -671,7 +671,10 @@ describe('getSchema', function () {
671671
assert.deepEqual(schema.sameAs, expectedSameAs);
672672
});
673673

674-
it('should escape special characters in social platform urls', function () {
674+
// Values are returned raw here; the ghost_head helper escapes them as JSON \u
675+
// escapes when serializing the JSON-LD block, so no HTML-entity escaping happens
676+
// at this layer (that would be indexed literally by structured-data consumers).
677+
it('should return social platform urls unescaped (escaping happens at JSON-LD serialization)', function () {
675678
const metadata = {
676679
site: {
677680
title: 'Site Title',
@@ -690,7 +693,7 @@ describe('getSchema', function () {
690693
}
691694
};
692695

693-
const expectedSameAs = buildExpectedSameAs('http://myblogsite.com/', {facebook: 'user&#x3D;name&#x3D;'});
696+
const expectedSameAs = buildExpectedSameAs('http://myblogsite.com/', {facebook: 'user=name='});
694697

695698
const schema = getSchema(metadata, data);
696699
assert.deepEqual(schema.sameAs, expectedSameAs);
@@ -960,7 +963,7 @@ describe('getSchema', function () {
960963
});
961964
});
962965

963-
it('should escape special characters in contributor social platform urls', function () {
966+
it('should return contributor social platform urls unescaped (escaping happens at JSON-LD serialization)', function () {
964967
const metadata = {
965968
...BASE_METADATA
966969
};
@@ -988,8 +991,8 @@ describe('getSchema', function () {
988991

989992
assertExists(schema.contributor);
990993
assert.deepEqual(schema.contributor[0].sameAs, [
991-
'http://coauthorsite.com/?user&#x3D;name&amp;param&#x3D;&lt;script&gt;alert(&quot;test&quot;)&lt;/script&gt;',
992-
'https://www.facebook.com/user&#x3D;name&#x3D;'
994+
'http://coauthorsite.com/?user=name&param=<script>alert("test")</script>',
995+
'https://www.facebook.com/user=name='
993996
]);
994997
});
995998
});

0 commit comments

Comments
 (0)