Skip to content

Commit b8f7032

Browse files
authored
fix: escape HTML in image, link, and media components (#2718)
1 parent f8ec7ac commit b8f7032

File tree

5 files changed

+54
-11
lines changed

5 files changed

+54
-11
lines changed

src/core/render/compiler/image.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getAndRemoveConfig } from '../utils.js';
1+
import { escapeHtml, getAndRemoveConfig } from '../utils.js';
22
import { isAbsolutePath, getPath, getParentPath } from '../../router/util.js';
33

44
export const imageCompiler = ({ renderer, contentBase, router }) =>
@@ -14,7 +14,7 @@ export const imageCompiler = ({ renderer, contentBase, router }) =>
1414
}
1515

1616
if (title) {
17-
attrs.push(`title="${title}"`);
17+
attrs.push(`title="${escapeHtml(title)}"`);
1818
}
1919

2020
if (config.size) {
@@ -42,7 +42,7 @@ export const imageCompiler = ({ renderer, contentBase, router }) =>
4242
url = getPath(contentBase, getParentPath(router.getCurrentPath()), href);
4343
}
4444

45-
return /* html */ `<img src="${url}" data-origin="${href}" alt="${text}" ${attrs.join(
45+
return /* html */ `<img src="${escapeHtml(url)}" data-origin="${escapeHtml(href)}" alt="${escapeHtml(text)}" ${attrs.join(
4646
' ',
4747
)} />`;
4848
});

src/core/render/compiler/link.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getAndRemoveConfig } from '../utils.js';
1+
import { escapeHtml, getAndRemoveConfig } from '../utils.js';
22
import { isAbsolutePath } from '../../router/util.js';
33

44
export const linkCompiler = ({
@@ -65,8 +65,8 @@ export const linkCompiler = ({
6565
}
6666

6767
if (title) {
68-
attrs.push(`title="${title}"`);
68+
attrs.push(`title="${escapeHtml(title)}"`);
6969
}
7070

71-
return /* html */ `<a href="${href}" ${attrs.join(' ')}>${text}</a>`;
71+
return /* html */ `<a href="${escapeHtml(href)}" ${attrs.join(' ')}>${text}</a>`;
7272
});

src/core/render/compiler/media.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { escapeHtml } from '../utils';
2+
13
export const compileMedia = {
24
markdown(url) {
35
return {
@@ -11,19 +13,19 @@ export const compileMedia = {
1113
},
1214
iframe(url, title) {
1315
return {
14-
html: `<iframe src="${url}" ${
16+
html: `<iframe src="${escapeHtml(url)}" ${
1517
title || 'width=100% height=400'
1618
}></iframe>`,
1719
};
1820
},
1921
video(url, title) {
2022
return {
21-
html: `<video src="${url}" ${title || 'controls'}>Not Supported</video>`,
23+
html: `<video src="${escapeHtml(url)}" ${title || 'controls'}>Not Supported</video>`,
2224
};
2325
},
2426
audio(url, title) {
2527
return {
26-
html: `<audio src="${url}" ${title || 'controls'}>Not Supported</audio>`,
28+
html: `<audio src="${escapeHtml(url)}" ${title || 'controls'}>Not Supported</audio>`,
2729
};
2830
},
2931
code(url, title) {

test/integration/example.test.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,9 @@ describe('Creating a Docsify site (integration tests in Jest)', function () {
228228
# Text between
229229
230230
[filename](_media/example3.js ':include :fragment=something_else_not_code')
231-
231+
232232
[filename](_media/example4.js ':include :fragment=demo')
233-
233+
234234
# Text after
235235
`,
236236
},
@@ -303,4 +303,26 @@ Command | Description | Parameters
303303
expect(mainText).toContain('Something');
304304
expect(mainText).toContain('this is include content');
305305
});
306+
307+
test.each([
308+
{ type: 'iframe', selector: 'iframe' },
309+
{ type: 'video', selector: 'video' },
310+
{ type: 'audio', selector: 'audio' },
311+
])('embed %s escapes URL for XSS safety', async ({ type, selector }) => {
312+
const dangerousUrl = 'https://example.com/?q="><svg/onload=alert(1)>';
313+
314+
await docsifyInit({
315+
markdown: {
316+
homepage: `[media](${dangerousUrl} ':include :type=${type}')`,
317+
},
318+
});
319+
320+
expect(
321+
await waitForFunction(() => !!document.querySelector(selector)),
322+
).toBe(true);
323+
324+
const mediaElm = document.querySelector(selector);
325+
expect(mediaElm.getAttribute('src')).toBe(dangerousUrl);
326+
expect(mediaElm.hasAttribute('onload')).toBe(false);
327+
});
306328
});

test/integration/render.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,16 @@ Text</p></div>"
241241
'"<p><img src="http://imageUrl" data-origin="http://imageUrl" alt="alt text" width="50" /></p>"',
242242
);
243243
});
244+
245+
test('escapes image alt and title to prevent attribute injection', async function () {
246+
const output = window.marked(
247+
'![alt" onerror="alert(1)](http://imageUrl \'title" onerror="alert(1)\')',
248+
);
249+
250+
expect(output).not.toContain(' onerror="alert(1)"');
251+
expect(output).toContain('alt="alt&quot; onerror=&quot;alert(1)"');
252+
expect(output).toContain('title="title&quot; onerror=&quot;alert(1)"');
253+
});
244254
});
245255

246256
// Headings
@@ -377,6 +387,15 @@ Text</p></div>"
377387
`"<p><a href="http://url" target="_blank" rel="noopener" id="someCssID">alt text</a></p>"`,
378388
);
379389
});
390+
391+
test('escapes link title to prevent attribute injection', async function () {
392+
const output = window.marked(
393+
`[alt text](http://url 'title" onclick="alert(1)')`,
394+
);
395+
396+
expect(output).not.toContain(' onclick="alert(1)"');
397+
expect(output).toContain('title="title&quot; onclick=&quot;alert(1)"');
398+
});
380399
});
381400

382401
// Skip Link

0 commit comments

Comments
 (0)