aFarkas / html5shiv

This script is the defacto way to enable use of HTML5 sectioning elements in legacy Internet Explorer.
http://paulirish.com/2011/the-history-of-the-html5-shiv/
9.88k stars 2.56k forks source link

html5shiv Vulnerability. #202

Open prakash-patel opened 8 years ago

prakash-patel commented 8 years ago

I use sonarQube for code analysis and it's gave me vulnerability for for html5shiv.js and html5shiv-printshiv.js file.

image

I updated code as below. Am i missing anything in below function.

ownerDocument.createDocumentFragment = function(h,f){
    return function(){ 
      var n=f.cloneNode(),c=n.createElement;
      h.shivMethods&&(
        // unroll the `createElement` calls
        getElements().join().replace(/[\w\-:]+/g, function(nodeName) {
          data.createElem(nodeName);
          data.frag.createElement(nodeName);
          return c("' + nodeName + '");
        }) 
      );
      return n;
    };
    }(html5, data.frag); 
  }

If you think this need to be change. I can create a pull request.

zg commented 8 years ago

Why don't you open the pull request and get feedback there instead?

prakash-patel commented 8 years ago

@zg I have created PR. I am not sure how to test my changes.

aborkowski commented 4 years ago

You are indeed missing something: The original Function constructor is used here precisely due to the evaluation of the string arguments (which is what SonarQube complains about). This is intended in order to "unroll the createElement calls". The original function body is generated as text in order to inline function calls and thereby unroll a loop which would be required otherwise (and cache the state at function creation), with one call to c() (and thereby n.createElement()) for each element. Your version calls the function c() with the string ' + nodeName + ' (literally) for each element instead. Hope this explains it. Have a look at the version of the code before the change in 189e939a925c53f3c8e76ae131882937980b8539.