Is "new Domparser" safer than "Document.createelement"?

Asked

Viewed 181 times

6

I created a script to try to remove unsafe content when injecting DOM (I’m using it in extensions/Addons for browsers):

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);
}

However, when submitting the extension to the http://addons.opera.com the moderator analyzed my code and sent me this message:

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

So I changed the code and it went 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);
}

Okay I made the change as requested, but I did not understand where this improved safety, it is just a curiosity level study.

What is the difference between the two in terms of safety?

javascript security-guard dom xss

  • I don’t understand enough and I admit I didn’t even read the question carefully. But what could be more unsafe than running code on the client? : ) If there is some insecurity in this code, there is nothing to do, because even if you do it right, someone goes and touches it and leaves you insecure. I understand the person wanting to make a JS code more robust, but more secure I find strange (unless it’s Node or something like that running on a server, then of course, it’s different). in this specific case may have made sense to run in the browser itself. In theory a plugin should not break through.

  • This response was generated automatically or someone "looked" at your code?

1 answer

3


Attributes of events such as onerror, onload and other things are executed even if the DOM element is not added to the page body, an example test:

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

See that you don’t have appendChild and yet onerror fires the alert().

Now look at it this way:

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

Look there was no shot from alert().

That’s the moment I do this:

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

 tmpDom = RemoveEls(tmpDom);

Even before executing the RemoveEls the innerHTML already triggers any event and in this may occur the security problem. Ie should use the DOMParser

Source: https://stackoverflow.com/a/37554728/1518921

Browser other questions tagged javascript security-guard dom xss

You are not signed in. Login or sign up in order to post.