Skip to content

Commit d930608

Browse files
authored
Merge pull request #244 from KManolov3/fix/uepr-224-svg-vulnerabilities
Fix/uepr 224 svg vulnerabilities
2 parents 5a5a5de + f010a81 commit d930608

File tree

3 files changed

+121
-4
lines changed

3 files changed

+121
-4
lines changed

packages/scratch-svg-renderer/src/sanitize-svg.js

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const DOMPurify = require('isomorphic-dompurify');
88

99
const sanitizeSvg = {};
1010

11+
const isInternalRef = ref => ref.startsWith('#') || ref.startsWith('data:');
12+
1113
DOMPurify.addHook(
1214
'beforeSanitizeAttributes',
1315
currentNode => {
1416

1517
if (currentNode && currentNode.href && currentNode.href.baseVal) {
1618
const href = currentNode.href.baseVal.replace(/\s/g, '');
1719
// "data:" and "#" are valid hrefs
18-
if ((href.slice(0, 5) !== 'data:') && (href.slice(0, 1) !== '#')) {
19-
20+
if (!isInternalRef(href)) {
21+
// TODO: Those can be in different namespaces than `xlink:`
2022
if (currentNode.attributes.getNamedItem('xlink:href')) {
2123
currentNode.attributes.removeNamedItem('xlink:href');
2224
delete currentNode['xlink:href'];
@@ -27,6 +29,24 @@ DOMPurify.addHook(
2729
}
2830
}
2931
}
32+
33+
// Remove url(...) usages with external references
34+
if (currentNode && currentNode.attributes) {
35+
for (let i = currentNode.attributes.length - 1; i >= 0; i--) {
36+
const attr = currentNode.attributes[i];
37+
const rawValue = attr.value || '';
38+
const value = rawValue.toLowerCase().replace(/\s/g, '');
39+
40+
const urlMatch = value.match(/url\((.+?)\)/);
41+
if (urlMatch) {
42+
const ref = urlMatch[1].replace(/['"]/g, '');
43+
if (!isInternalRef(ref)) {
44+
currentNode.removeAttribute(attr.name);
45+
}
46+
}
47+
}
48+
}
49+
3050
return currentNode;
3151
}
3252
);
@@ -37,13 +57,34 @@ DOMPurify.addHook(
3757
if (data.tagName === 'style') {
3858
const ast = parse(node.textContent);
3959
let isModified = false;
40-
// Remove any @import rules as it could leak HTTP requests
60+
4161
walk(ast, (astNode, item, list) => {
42-
if (astNode.type === 'Atrule' && astNode.name === 'import') {
62+
// @import rules
63+
if (astNode.type === 'Atrule' && astNode.name.toLowerCase() === 'import') {
4364
list.remove(item);
4465
isModified = true;
4566
}
67+
68+
// Elements using url(...) for external resources
69+
if (astNode.type === 'Declaration' && astNode.value) {
70+
let shouldRemove = false;
71+
walk(astNode.value, valueNode => {
72+
if (valueNode.type === 'Url') {
73+
const urlValue = (valueNode.value.value || '').trim().replace(/['"]/g, '');
74+
75+
if (!isInternalRef(urlValue)) {
76+
shouldRemove = true;
77+
}
78+
}
79+
});
80+
81+
if (shouldRemove) {
82+
list.remove(item);
83+
isModified = true;
84+
}
85+
}
4686
});
87+
4788
if (isModified) {
4889
node.textContent = generate(ast);
4990
}
Lines changed: 25 additions & 0 deletions
Loading
Lines changed: 51 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)