Not working correctly with latest RetireJS repo

[chadlwilson]

Since the commit at https://github.com/RetireJS/retire.js/commit/60ffbeb1523c503da886b2a47b39551697ed06c8 the RetireJSRepository here cannot understand filecontent on the new entry correctly, yielding.

java.util.regex.PatternSyntaxException: Unclosed character class near index 79
u.version="([0-9][0-9.a-z_\\\\-]+)";u.settings=[];u.models=\{\};u.models.oSearch
                                                                               ^
        at java.base/java.util.regex.Pattern.error(Pattern.java:2028)
        at java.base/java.util.regex.Pattern.clazz(Pattern.java:2690)
        at java.base/java.util.regex.Pattern.sequence(Pattern.java:2139)
        at java.base/java.util.regex.Pattern.expr(Pattern.java:2069)
        at java.base/java.util.regex.Pattern.compile(Pattern.java:1783)
        at java.base/java.util.regex.Pattern.<init>(Pattern.java:1430)
        at java.base/java.util.regex.Pattern.compile(Pattern.java:1069)
        at com.h3xstream.retirejs.repo.VulnerabilitiesRepository.findByFileContent(VulnerabilitiesRepository.java:117)
        at com.h3xstream.retirejs.repo.ScannerFacade.scanScript(ScannerFacade.java:125)

There is a downstream issue at https://github.com/jeremylong/DependencyCheck/issues/4695 caused by this on latest RetireJS repo.

It seems to be due to the [] in the filecontent value which would need to be \-escaped in regex. I cannot see consistent regex escaping that is attempted by the code at the below, so perhaps it is not expecting arbitrary content in the expressions?

https://github.com/h3xstream/burp-retire-js/blob/1d9b7dc7d6a53325b8d3db93c9e5fe1290883c1e/retirejs-core/src/main/java/com/h3xstream/retirejs/repo/VulnerabilitiesRepository.java#L103-L133

https://github.com/h3xstream/burp-retire-js/blob/1d9b7dc7d6a53325b8d3db93c9e5fe1290883c1e/retirejs-core/src/main/java/com/h3xstream/retirejs/util/RegexUtil.java#L65-L75

Is "filecontent" : [ "http://www.datatables.net\n +DataTables (§§version§§)", "u.version=\"(§§version§§)\";u.settings=[];u.models={};u.models.oSearch" ], valid from the library's perspective?

[bbossola]

I created a simple fix for the issue, see https://github.com/h3xstream/burp-retire-js/pull/74 It basically escapes the specific case and also introduce a simple logic to ignore future unparsable regex, after logging those, so that we won't have the same disruption in the future.

[chadlwilson]

Looks like retireJS have changed the expression at https://github.com/RetireJS/retire.js/commit/7007575f637b840016edf8a9512d608f00795b1b as of a few minutes ago, so this is working again with the default retire JS repository content again now.

Will leave this open in case discussion is needed on whether there is still change needed within this lib.

[bbossola]

I would suggest we have at least the safeguard in place to avoid another breaking happening the same way in the future. We do not have any control on the upstream regexes. which are loaded automatically and outside the control of the library. I believe we should be resilient to a breaking change from the RetireJS json feeds.

[bbossola]

Please let me know if you prefer to accept the PR in this format or if you want me to leave only the defensive logic. In that case I will update the PR gladly.

[bbossola]

Any other opinon about this issue? My PR https://github.com/h3xstream/burp-retire-js/pull/74 is there and can be merged, but I am also happy to change it in any other suitable way.

[bbossola]

bump :)

[bbossola]

Is there any update about this one?

[bbossola]

bump :)

[chadlwilson]

As #74 was merged I'll close this for now. I guess we need a release and an update inside OWASP dep check though.