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

Security Issue #107

Open
allanice001 opened this issue Apr 17, 2021 · 18 comments
Open

Security Issue #107

allanice001 opened this issue Apr 17, 2021 · 18 comments

Comments

@allanice001
Copy link

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
Copy link

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

@dmitrizzle
Copy link

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
Copy link
Author

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

@hthetiot
Copy link

Thank you @allanice001

@julianlam
Copy link

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

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

@arnauddsj
Copy link

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
Copy link

hthetiot commented Jan 16, 2022

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
Copy link
Author

allanice001 commented Jan 17, 2022

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
Copy link

Martii commented Mar 14, 2022

@it-smtech
Copy link

Any updates on this issue please ?

@allanice001
Copy link
Author

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

still no feedback from the original maintainer @jaredhanson

@Dylankjy
Copy link

Dylankjy commented Nov 5, 2022

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

@allanice001
Copy link
Author

allanice001 commented Nov 5, 2022

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
Copy link

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

@allanice001
Copy link
Author

@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: #107 (comment)

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
Copy link

@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
Copy link
Author

allanice001 commented Nov 5, 2022

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
Copy link

hthetiot commented Nov 23, 2022

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.

allanice001 added a commit to passportjs/passport-twitter that referenced this issue Nov 24, 2022
Update the install command to point to the npm home of this repo instead of the original one

Fixes jaredhanson/passport-twitter#107
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants