Reputation: 1419
I wonder if someone would be able to confirm whether this is safe or not, as I can't seem to find any reference online, nor another question on SO, that specifically deals with this "solution" to XSS.
I need to embed rich-text comments into my page. Obviously the risks of XSS are high, so my plan was to innerHTML a temporary DIV in a DocumentFragment and then recurse the tree, using a pre-defined whitelist of tagNames and attribute names that I have deeemed 'safe', removing any unsafe ones. I can then move this now-safe HTML into my real document.
Is this a safe way of doing it? Is there any way an XSS attack could be triggered by doing this through a DocumentFragment? I'm hoping that it'll be isolated from the real document and hence shielded from user event triggering that might kick off any attacks.
Upvotes: 2
Views: 895
Reputation: 21766
I wouldn't recommend to write your own anti-XSS library as malicious users are bound to know an exploit that you haven't accounted for. I would recommend to use a third party library, for example Google Caja HTML Sanitiser.
Having looked at your Pen, you code is still vulnerable if the <
and >
tags are escaped:
var unsafe = '\u003Cimg src=1 onerror=alert(\u0027XSS_attack_in_progress\u0027)\u003E',
//var unsafe = '<h3>Hello</h3><h4>World</h4>',
whitelistTags = ['h1', 'h2', 'h3', 'b', 'i', 'em', 'strong', 'u'],
testNode = document.getElementById('testNode');
makeSafeAndAddToDoc(unsafe, testNode);
function makeSafeAndAddToDoc(unsafe, targetParent) {
var safeDocFrag = document.createDocumentFragment(),
containerDiv = safeDocFrag.appendChild(document.createElement("DIV")),
nextChild;
containerDiv.innerHTML = unsafe;
while ((nextChild = containerDiv.firstChild)) {
if (isSafe(nextChild)) {
safeDocFrag.appendChild(containerDiv.firstChild);
console.debug(safeDocFrag.children);
} else {
containerDiv.removeChild(nextChild);
}
}
safeDocFrag.removeChild(containerDiv);
targetParent.appendChild(safeDocFrag);
}
function isSafe(testNode) {
var tag = testNode.tagName && testNode.tagName.toLowerCase(),
isTextNode = testNode.nodeType === 3;
if (!isTextNode && whitelistTags.indexOf(tag) === -1) {
console.warn('Removing unsafe element: ', testNode.tagName);
return false;
}
for (var i = 0; i < testNode.childNodes.length; i++) {
if (!isSafe(testNode.childNodes[i])) {
testNode.removeChild(testNode.childNodes[i]);
i--;
}
}
return true;
}
#testNode {
min-width: 10px;
min-height: 10px;
border: 1px solid red;
}
<div id="testNode"></div>
Upvotes: 2