MicrosoftEdge / static-code-scan

Run this quick static code scan on any URL to check for out-of-date libraries, layout issues and accessibility.
Other
1.05k stars 228 forks source link

jQuery false negative (failed test) version check on bootstrap.js file #97

Closed m-gagne closed 9 years ago

m-gagne commented 9 years ago

Description

The static code scanner is returning a false negative when scanning the source code for bootstrap.min.js version 3.3.4 against the site http://www.gotsomething.com

The tool will report that jQuery is detected with version 3.3.4 source line number 539 and that the recommended resolution is to

The jQuery test inappropriately identifies bootstrap.js as jQuery and incorrectly uses the version in the header (3.3.4)

The issue is the regular expression at line 284 in lib\checks\check-libs.js

    //If header fails, we look with another pattern
    var regex = /(?:jquery[,\)].*=")(\d+\.\d+)(\..*?)"/gi;

The issue is the .*=" part of the match because what is happening is if the source code contains jquery, or jquery( then ANYTHING inbetween and finally a =0.0 (where 0 is any digit) it will match. You can see how this resulted in a false negative here: https://regex101.com/r/nQ6rA3/1

Further Details

Site being scanned: http://www.gotsomething.com Failed Test Result:

    {
    "testName": "jslibs",
    "url": "http://gotsomething.com/",
    "passed": false,
    "data": [
            {
            "name": "jQuery",
            "needsUpdate": true,
            "minVersion": "1.6.4",
            "version": "3.3.4",
            "url": "http://gotsomething.com/wp-content/themes/materialwp-materialwp/bower_components/bootstrap/dist/js/bootstrap.min.js?ver=3.3.4",
            "lineNumber": 539
            }
    ]
    }

JavaScript Source Url: http://gotsomething.com/wp-content/themes/materialwp-materialwp/bower_components/bootstrap/dist/js/bootstrap.min.js?ver=3.3.4

m-gagne commented 9 years ago

After some more testing, the issue is the minified version of bootstrap.js that places everything on one line, it works fine against the normal (non-minified) version of bootstrap.js

m-gagne commented 9 years ago

Not a perfect fix, but this would likely do the trick (limit between 0 and 200 characters):

/(?:jquery[,)].{0,200}=")(\d+.\d+)(..*?)"/gi

molant commented 9 years ago

nice @m-gagne, do you mind doing a PR for this?

Thanks a lot!

m-gagne commented 9 years ago

Happy to submit a PR, however seems like the unit tests are broken?

Running "nodeunit:all" (nodeunit) task
Testing altImg_test.js.F..
>> AltImg - Empty alt attribute
>> Message: http://localhost:1338/altImg-2.html passed: true !== false
>> Error: true == false
>> at C:\Users\marcg\Code\static-code-scan\test\altImg_test.js:47:22
molant commented 9 years ago

Yup, need to find time to fix them: https://github.com/MicrosoftEdge/static-code-scan/issues/95

m-gagne commented 9 years ago

Would you happen to know a source file that was used to test that particular regex? Just want to make sure my fix will still work :)

molant commented 9 years ago

If I remember correctly I used the minified and unminified versions of jQuery for a bunch of versions (I think that starting from 1.4.0).

molant commented 9 years ago

Fixed via 45bba60dca3d41c1daa3baacbc4c2e196e7d1b0a