Skip to content

Commit 827daa9

Browse files
committed
fix: escape HTML attributes in image and link components to prevent XSS vulnerabilities
1 parent fd795a2 commit 827daa9

4 files changed

Lines changed: 47 additions & 5 deletions

File tree

src/core/render/compiler/image.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -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) {
@@ -28,6 +28,7 @@ export const imageCompiler = ({ renderer, contentBase, router }) =>
2828

2929
if (config.class) {
3030
let classes = config.class;
31+
console.log(classes);
3132
if (Array.isArray(config.class)) {
3233
classes = config.class.join(' ');
3334
}
@@ -42,7 +43,7 @@ export const imageCompiler = ({ renderer, contentBase, router }) =>
4243
url = getPath(contentBase, getParentPath(router.getCurrentPath()), href);
4344
}
4445

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

src/core/render/compiler/link.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export const linkCompiler = ({
6565
}
6666

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

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

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)