Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Blackduck Vulnerabilities found in serenity js #1062

Closed
nbarrett opened this issue Nov 25, 2021 · 7 comments
Closed

Project Blackduck Vulnerabilities found in serenity js #1062

nbarrett opened this issue Nov 25, 2021 · 7 comments

Comments

@nbarrett
Copy link
Contributor

nbarrett commented Nov 25, 2021

Hi @jan-molak ! Long time no speak. It seems that my current project is considering moving to Serenity-js + WebdriverIO but in order to do that, we need to assess the vulnerability impact. So our team has run v2.32.3 through our internal blackduck scanning and have found the following Vulnerabilities:
image

And the following Operational Risks:

image

Right now we are focussed on the vulnerabilities and are wondering whether it might be possible to upgrade the project dependencies to resolve these? Maybe I can help with a PR, once I work out where these dependencies are coming from. I enclose an excel sheet of the affected libraries and the specific vulnerabilities:

blackduck-vulnerability-report.xlsx

I look forward to hearing from you and hopefully contributing....

Thanks, @nbarrett

@jan-molak
Copy link
Member

jan-molak commented Nov 25, 2021

Hey @nbarrett! Sure thing, I've just released 2.32.4 with updated dependencies, so it would be good to know what Blackduck thinks about those?

I don't think Serenity/JS depends on any of the libs from your Excel spreadsheet directly, but if you could figure out what's pulling them in I'm sure we can figure out how to upgrade them.

Also, out of curiosity, can you run Blackduck on a branch as opposed to the released version? Might be good to see if there are any potential issues with the upcoming 3.0 release on branch features/web

@nbarrett
Copy link
Contributor Author

nbarrett commented Nov 30, 2021

Hi @jan-molak ! - thanks for your quick response - I've just got our security team to re-run the blackduck scan on Serenity-js-2.32.4 and it seems to have exactly the same vulnerabilities as the prior version. I have captured an additional column showing how the vulnerability was matched and included that as a new Matched from column and attach here - hope this helps:

blackduck-vulnerability-report.xlsx

I've not had a chance to perform the same on the 3.0 branch yet. We'll have a look to see how easy it is to do that... Cheers 👍

@jan-molak
Copy link
Member

jan-molak commented Dec 9, 2021

Hi @nbarrett - 3.0.0-rc.x is now on npm.

And I have also done a bit of research via npm ls <module-name>

json-schema 0.2.3

This is pulled in via http-signature, which is a transitive dependency of @serenity-js/local-server DEV dependency - restify (not used in the production code):

└─┬ restify@8.6.0 extraneous
  └─┬ http-signature@1.2.0
    └─┬ jsprim@1.4.1
      └── json-schema@0.2.3
  • To fix this issue properly, restify would need a PR to update http-signature to 1.3.6 or newer, since that module pulls in a newer jsprim, which then depends on json-schema 0.4.0.

  • To work around it, @serenity-js/local-server could override the version of http-signature since resify depends on a version range.

yargs-parser 2.4.1

This is pulled in via instanbul-merge, which is a DEV dependency used as part of the Serenity/JS CI/CD pipeline (not used in the production code):

├─┬ istanbul-merge@1.1.1
│ └─┬ yargs@4.8.1
│   └── yargs-parser@2.4.1
  • To fix this issue properly, istanbul-merge would need a PR to update yargs to the latest version.

  • I don't see any obvious workaround.

lodash 4.17.21

This is the latest version of lodash, which is used by several DEV dependencies and the vulnerability is being disputed.

However, lodash is also used by port-finder 2.x (via async), which is a PROD dependency of @serenity-js/local-server.

  • To fix this issue properly, portfinder needs a PR to update async to 3.x, which doesn't depend on lodash

ansi-regex, nth-check, css-what, minimist

These are transitive dependencies of several DEV dependencies pulled in via metalsmith and esdoc, used to build the static HTML for the serenity-js.org website. Those modules are not used in the Serenity/JS production code.

However, since ESDoc is (sadly) no longer maintained, Serenity/JS website should probably get migrated to something like Docusaurus.


So to summarise, apart from lodash 4.17.21 used by @serenity-js/local-server, neither one of those dependencies is used in the production code.

However, to make the world a better place I think we should offer a helping hand to portfinder, istanbul-merge and restify 😊

Would you be open to helping out with that?

@nbarrett
Copy link
Contributor Author

Hi @jan-molak - sure thing - I'll raise those 3 PRs forthwith - thanks for your analysis!

nbarrett pushed a commit to nbarrett/node-restify that referenced this issue Dec 20, 2021
- Helps address vulnerability in serenity-js as part of [issue restify#1062](serenity-js/serenity-js#1062 (comment))
nbarrett pushed a commit to nbarrett/istanbul-merge that referenced this issue Dec 20, 2021
- Helps address vulnerability in serenity-js as part of [issue #1062](serenity-js/serenity-js#1062 (comment))
@nbarrett
Copy link
Contributor Author

nbarrett commented Dec 20, 2021

Hi @jan-molak - I've raised 2 of the 2 PRs you suggested but having trouble with bumping the async dependency on portfinder from ^2.6.2 to ^3.2.2. When I update the npm dependencies and run npm run test I get this error in vows (the test framework for the project):

> portfinder@1.0.28 test /Users/nickbarrett/dev/node-portfinder
> vows test/*-test.js --spec

 
  ♢ portfinder 
  
/Users/nickbarrett/dev/node-portfinder/node_modules/vows/lib/vows.js:208
        util.puts(console.result(results));
             ^

TypeError: util.puts is not a function
    at process.<anonymous> (/Users/nickbarrett/dev/node-portfinder/node_modules/vows/lib/vows.js:208:14)
    at process.emit (events.js:314:20)
    at process.options.Emitter.emit (/Users/nickbarrett/dev/node-portfinder/node_modules/vows/lib/vows.js:241:24)

This looks to me to be a problem with using a deprecated library util in node but not too sure how to upgrade this as a change now looks to be needed in vows which is already at the latest version. Any suggestions appreciated please! 🤞 It might be that I'm using a version of node that is too high for the projects I'm creating PRs for as the portfinder is currently set to a very low value of:

"engines": {
    "node": ">= 0.12.0"
  },

@jan-molak
Copy link
Member

jan-molak commented Dec 20, 2021

Thanks, @nbarrett!

I had a quick look at portfinder. It looks like its CI pipeline supports 0.12, 4.2 and 5.0, so given 4.2 is the only LTS in that mix, I'd try with that for the PR.

It seems like @eriktrom is interested in dropping support for the ancient Node 0.10.x altogether (http-party/node-portfinder#122), but that's something for a separate PR I think. The maintainers did mention that the project is locked for any new features (http-party/node-portfinder#117 (comment)), but I'm hoping that they'll accept a security fix.

@jan-molak
Copy link
Member

Hey @nbarrett!

I think we can close this ticket now, as at the time of writing Serenity/JS:

  • depends on restify 11.1.0, which no longer depends on the vulnerable json-schema
  • no longer depends on istanbul-merge,
  • depends on the latest version of portfinder, which updated its dependency on async in Update "async" dependency http-party/node-portfinder#126
  • no longer depends on ESDoc, which means it doesn't have to depend on ansi-regex, nth-check, css-what, or minimist

Please let me know if you come across any other issues in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants