search

orliesaurus / nodemailer-mailgun-transport

nodemailer is an amazing node module to send emails within any of your nodejs apps. This is the transport plugin that goes with nodemailer to send email using Mailgun 🔫
MIT License
880 stars 97 forks source link

high severity vulnerability in netmask plugin #103

Closed zhyrin closed 3 years ago

zhyrin commented 3 years ago

  1. What kind of issue are you reporting?

    • A bug in a plugin of nodemailer-mailgun-transport ^2.0.2 (netmask plugin)
    • High severity vulnerability

  2. State your problem here: Run "npm audit" or "npm install" with nodemailer-mailgun-transport ^2.0.2 gives me: "netmask npm package vulnerable to octal input data" "patched in >=2.0.1"

Netmask is used by up to 300k live projects. Vulnerability reported on about two days ago.

orliesaurus commented 3 years ago

Hey, I tried to patch the package-lock.json to use a NON-VULNERABLE version of netmask - I think it worked? Let me know what you're seeing now :)

anachronic commented 3 years ago

Looks fixed to me in a clean checkout + npm install

I'm still seeing it in a production project through yarn audit, though, I suspect it hasn't been published

orliesaurus commented 3 years ago

@anachronic I published in npm as 2.0.3 - https://www.npmjs.com/package/nodemailer-mailgun-transport

rfox12 commented 3 years ago

Apparently nodemailer-mailgun-transport id using a now non-official mailgun library? Is this an easy swap to the "official" version? Seems like the proper dependency should be "mailgun.js" and then that will alleviate the security vulnerability (and hopefully help for future issues). Discussion here: https://github.com/mailgun/mailgun-js/issues/122

zhyrin commented 3 years ago

Yeah just change the dependency and rewrite the code bit of code that are used for the official library. official plugin: https://www.npmjs.com/package/mailgun.js deprecated plugin: https://www.npmjs.com/package/mailgun-js (its referenced in mailgun docs, hence the confusion)

official uses '...require=('mailgun.js) as opposed to ...require('mailgun-js')

Tired, but hope it was a tldr

orliesaurus commented 3 years ago

@zhyrin guess someone is gonna have to do some research to see how much code rewrite is needed

zhyrin commented 3 years ago

not interested

anachronic commented 3 years ago

@zhyrin guess someone is gonna have to do some research to see how much code rewrite is needed

Is #104 not a fix for this?

orliesaurus commented 3 years ago

not interested

great! at least you're honest about how you like open source

orliesaurus commented 3 years ago

Is #104 not a fix for this?

let me take a look

omerlh commented 3 years ago

Look like the latest version of mail gun is still vulnerable? https://snyk.io/advisor/npm-package/mailgun-js

orliesaurus commented 3 years ago

@omerlh We've upgraded to the other one - that one is deprecated

omerlh commented 3 years ago

Which one? I can see the latest version is still vulnerable: https://snyk.io/advisor/npm-package/nodemailer-mailgun-transport

anachronic commented 3 years ago

I believe this is solved in master (see #104), however, 2.0.3 (last version published) seems to point to the last commit before the fix.

Although I'm not sure, maybe publishing a new version solves this?

shawncarr commented 3 years ago

@orliesaurus can we get a new release based on @anachronic's point?

orliesaurus commented 3 years ago

Yeah I definitely need to push the update that is in master! Thanks y'all!

axelauvinen commented 3 years ago

Any timeline on this version bump? Should this ticket be closed before the latest version is updated to master?