Is "new DOMParser" safer than "document.createElement"?

4

I created a script to try to remove unsafe content at the time of injecting DOM (I'm using in extensions / addons for browsers):

var str = "<strong>Hello</strong> mundo <script src="http://site/badscript.js"></script>";CreateDOM(str);functionRemoveAttrs(target){varattrs=target.attributes,currentAttr;varvalidAttrs=["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);
}

However, when submitting the extension to the link the moderator reviewed my code and sent me this message:

  

Your cleanDomString method is not safe, please replace:   tmpDom.innerHTML = date; with: var tmpDom = (new   DOMParser) .parseFromString (data, "text / html"). Body;

     

and remove: var tmpDom = document.createElement ("div");

     

or use:    link

     

dmichnowicz; May 30, 2016 8:46:57 AM UTC

So I changed the code and it looked like this:

var str = "<strong>Hello</strong> mundo <script src="http://site/badscript.js"></script>";CreateDOM(str);functionRemoveAttrs(target){varattrs=target.attributes,currentAttr;varvalidAttrs=["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);
}

Alright, I made the change as a request, but I did not understand where this improved security, it's just a curiosity at the study level.

What's the difference between the two in terms of security?

    
asked by anonymous 30.05.2016 / 18:54

1 answer

2

Event attributes such as onerror , onload , and other things run even if the DOM element is not added to the body of the page, a test example:

function createDOM(str) {
  document.createElement("div").innerHTML = str;
}
createDOM('<img src="//" onerror="alert(\'Executou mesmo sem adicionar ao document.body!\')" />');

See that it does not have appendChild and even onerror triggers alert() .

Now look at this:

function createDOM(str) {
  new DOMParser().parseFromString(str, "text/html").body;
}
createDOM('<img src="//" onerror="alert(\'Executado\')" />');

There was no firing of alert() .

That is when I do this:

 var tmpDom = document.createElement("div");
 tmpDom.innerHTML = MinhaString;

 tmpDom = RemoveEls(tmpDom);

Even before running RemoveEls innerHTML already triggers any event and in this the security problem may occur. That is, you should use DOMParser

    

31.05.2016 / 22:31