search

jaredhanson / passport-twitter

Twitter authentication strategy for Passport and Node.js.
https://www.passportjs.org/packages/passport-twitter/?utm_source=github&utm_medium=referral&utm_campaign=passport-twitter&utm_content=about
MIT License
469 stars 130 forks source link

Security Issue #107

Open allanice001 opened 3 years ago

allanice001 commented 3 years ago

Issues with no direct upgrade or patch: ✗ XML External Entity (XXE) Injection [Medium Severity][https://snyk.io/vuln/SNYK-JS-XMLDOM-1084960] in xmldom@0.1.31 introduced by passport-twitter@1.0.4 > xtraverse@0.1.0 > xmldom@0.1.31 This issue was fixed in versions: 0.5.0

Any chance of upgrading the xmldom dependency?

hthetiot commented 3 years ago

The fix need to append on xtraverse first, see PR: https://github.com/jaredhanson/node-xtraverse/pull/1

dmitrizzle commented 3 years ago

The fix need to append on xtraverse first, see PR: jaredhanson/node-xtraverse#1

This lib looks to have last been updated 8 years ago. Perhaps it needs to be forked, or is there any other way to fix this issue here?

allanice001 commented 3 years ago

I've done the needful - https://www.npmjs.com/package/xtraverse1

hthetiot commented 3 years ago

Thank you @allanice001

julianlam commented 2 years ago

@jaredhanson I know you're around now, you committed to this repo yesterday :smile_cat:

Can you merge jaredhanson/node-xtraverse#1 now? :pray:

arnauddsj commented 2 years ago

Hello, any news on this matter? I implemented "passport-twitter": "^1.0.4", today and discover that it Depends on vulnerable versions of xmldom. Is it a package people use in production or is there another package somewhere to be used? Thanks!

hthetiot commented 2 years ago

Hello, any news on this matter? I implemented "passport-twitter": "^1.0.4", today and discover that it Depends on vulnerable versions of xmldom. Is it a package people use in production or is there another package somewhere to be used? Thanks!

If you take the time to understand the previous comments in this issue you should understand that the fix is in the way. And asking for an alternative package instead of helping the maintainer to update is not the right way to help an open source librairie in my POV. So be patiente is the best course of action if you are not knowledgeable .

allanice001 commented 2 years ago

I love OSS, but this issue has been going on for over 6 months, without the current maintainer @jaredhanson taking note of this issue., so in all respect @hthetiot - please advise an alternative - the PRs are done, and unless there are more maintainers to the package, there really is no way forward.

Martii commented 2 years ago

Ref:

it-smtech commented 2 years ago

Any updates on this issue please ?

allanice001 commented 2 years ago

I've done the needful - https://www.npmjs.com/package/xtraverse1

still no feedback from the original maintainer @jaredhanson

Dylankjy commented 2 years ago

Good day, is there an alternative package to use? Seems that the maintainer is unresponsive for a very long time.

allanice001 commented 2 years ago

Alright everyone, I've cloned the two repositories and started their own lifecycle at passportjs I've also created a npm org @passport-js

Right now, you're more than welcome to look at using @passport-js/passport-twitter as a replacement dependency, which consumes the patched traverse package - @passport-js/xtraverse

julianlam commented 2 years ago

@allanice001 are you part of the passport.js organization?

allanice001 commented 2 years ago

@julianlam I am a tenured developer and security professional with over 20 years of experience. the repository owner has not responded to what is IMHO a critical bug in over a year, and everyone here is just pinging a dead thread.

as @dmitrizzle suggested here: https://github.com/jaredhanson/passport-twitter/issues/107#issuecomment-913200011

What I have done is exactly that. clone the repository, and create the new lifecycles. Something it seems like no one else wanted to do.

The GitHub and npm organizations I mentioned are created by myself at the minute, and would love to have maintainers join. I'd be happy to continue this conversation offline or on a different platform

julianlam commented 2 years ago

@allanice001 thanks for responding, I only raise the concern due to concerns over the software supply chain. It was not meant as an attack on your person 🙂

I was just surprised that you elected to use the passportjs name instead of your own personal fork.

allanice001 commented 2 years ago

no worries - the intention is that a community can be built to support it, with multiple leaders, instead of just a single person, and that's what led to my decision not to point to a personal fork.

In the same breath, what happens when I don't respond to security concerns or concerns regarding the maintenance of the packages or other issues that are bound to happen in the future? How does open source survive if it's just one person?

In no way am I trying to nullify what Jared has started, but more looking at a way to improve the status quo for everyone

hthetiot commented 1 year ago

Thank you @allanice001, I was hoping to avoid fork and motivate the maintainer, I was going to fork myself in the end and re-publish on npm, having it under @passportjs umbrella is even better.

Let all move on to https://www.npmjs.com/package/@passport-js/passport-twitter

Cold case close for me.