I create a script for try remove insecure content (I'm using it for browser extensions):
var str = "<strong>Hello</strong> mundo <script src="http://site/badscript.js"></script>";
CreateDOM(str);
function RemoveAttrs(target)
{
var attrs = target.attributes, currentAttr;
var validAttrs = [ "href", "class", "id", "target" ];
for (var i = attrs.length - 1; i >= 0; i--) {
currentAttr = attrs[i].name;
if (attrs[i].specified && validAttrs.indexOf(currentAttr) === -1) {
target.removeAttribute(currentAttr);
}
if (
currentAttr === "href" &&
/^(#|javascript[:])/gi.test(target.getAttribute("href"))
) {
target.parentNode.removeChild(currentAttr);
}
}
}
function RemoveEls(target)
{
var current;
//Remove elements insecure (blacklist)
var list = target.querySelectorAll("script,link,...");
for (var i = list.length - 1; i >= 0; i--) {
current = list[i];
current.parentNode.removeChild(current);
}
//Remove insecure attributes (whitelist)
list = target.getElementsByTagName("*");
for (i = list.length - 1; i >= 0; i--) {
RemoveAttrs(list[i]);
}
return target;
}
function CreateDOM(MinhaString)
{
var tmpDom = document.createElement("div");
tmpDom.innerHTML = MinhaString;
tmpDom = RemoveEls(tmpDom);
//Inject in container
document.getElementById("container").appendChild(tmpDom);
}
I'm using this script in an addon I created for opera and Google Chorme, however the site moderator ("addons.opera.com") said to me this:
Your cleanDomString method is not safe, please replace: tmpDom.innerHTML = data; with: var tmpDom = (new DOMParser).parseFromString(data, "text/html").body;
and remove: var tmpDom = document.createElement("div");
or use: https://github.com/operatester/safeResponse/blob/1.1/safeResponse.js
dmichnowicz; May 30, 2016 8:46:57 AM UTC
The code looks like this:
var str = "<strong>Hello</strong> mundo <script src="http://site/badscript.js"></script>";
CreateDOM(str);
function RemoveAttrs(target)
{
var attrs = target.attributes, currentAttr;
var validAttrs = [ "href", "class", "id", "target" ];
for (var i = attrs.length - 1; i >= 0; i--) {
currentAttr = attrs[i].name;
if (attrs[i].specified && validAttrs.indexOf(currentAttr) === -1) {
target.removeAttribute(currentAttr);
}
if (
currentAttr === "href" &&
/^(#|javascript[:])/gi.test(target.getAttribute("href"))
) {
target.parentNode.removeChild(currentAttr);
}
}
}
function RemoveEls(target)
{
var current;
//Remove elements insecure (blacklist)
var list = target.querySelectorAll("script,link,...");
for (var i = list.length - 1; i >= 0; i--) {
current = list[i];
current.parentNode.removeChild(current);
}
//Remove insecure attributes (whitelist)
list = target.getElementsByTagName("*");
for (i = list.length - 1; i >= 0; i--) {
RemoveAttrs(list[i]);
}
return target;
}
function CreateDOM(MyString)
{
var tmpDom = (new DOMParser).parseFromString(MyString, "text/html").body;
tmpDom = RemoveEls(tmpDom);
//Inject in container
document.getElementById("container").appendChild(tmpDom);
}
I made the change, but I would like to understand what it became safer my code. For me they both seem to do the same thing.
What are the differences (terms of their safety)?
Effectively, your current code is not safe. innerHTML
doesn't run scripts in created <script>
elements, but it does run event handler content attributes.
function createDOM(str) {
document.createElement("div").innerHTML = str;
}
createDOM('<img src="//" onerror="console.log(\'You are pwned!\')" />');
function createDOM(str) {
new DOMParser().parseFromString(str, "text/html");
}
createDOM('<img src="//" onerror="console.log(\'You are safe\')" />');
However, note that DOMParser
provides safety if you only want to manipulate the DOM elements from an untrusted HTML string. It's like a sandbox. But if then you get these elements and append it in the current document, they will still be able to run JS.
function createDOM(str) {
document.body.appendChild(new DOMParser().parseFromString(str, "text/html").body);
}
createDOM('<img src="//" onerror="console.log(\'You are pwned!\')" />');
If you really need something like this, I would use a small whitelist of allowed elements and attributes, and get rid of everything else.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With