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

Dependency nodemailer-smtp-transport is redundant and introduces flagged critical vulnerability #489

Open
robertdeniszczyc2 opened this issue Jan 8, 2025 · 0 comments

Comments

@robertdeniszczyc2
Copy link

Hi,

If you consume HOF in a project building with npm, there is a nested dependency vulnerability flagged which is ranked "Critical". The vulnerability is:

Arbitrary Code Execution in underscore - GHSA-cf4h-3jhx-xvhq

The issue is in the version of underscore which is nested within the nodemailer-smtp-transport dependency:

├─┬ [email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └── [email protected]

This issue is not flagged when installing with yarn and running a yarn audit. It only flags with npm install and npm audit.

From investigation, it appears the vulnerability could be a false positive as the code path allegedly isn't executed and therefore there are no plans to update nodemailer-smtp-transport to resolve the issue: nodemailer/nodemailer-smtp-transport#34 (comment)

However I've done further research into the usage of nodemailer-smtp-transport and I think it could be redundant in HOF, and just use the nodemailer dep which is already included in the project: https://nodemailer.com/smtp/

I have run a test on a local branch making the following changes and all the tests pass OK:

components/emailer/transports/smtp.js:

diff --git a/components/emailer/transports/smtp.js b/components/emailer/transports/smtp.js
index 8fdc1fe..68584b4 100644
--- a/components/emailer/transports/smtp.js
+++ b/components/emailer/transports/smtp.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const smtp = require('nodemailer-smtp-transport');
+const smtp = require('nodemailer');

package.json:

diff --git a/package.json b/package.json
index 65ac162..114735a 100644
--- a/package.json
+++ b/package.json
@@ -78,7 +78,6 @@
     "mustache": "^4.2.0",
     "nodemailer": "^6.6.3",
     "nodemailer-ses-transport": "^1.5.1",
-    "nodemailer-smtp-transport": "^2.7.4",

test/components/emailer/transports/smtp.spec.js:

diff --git a/test/components/emailer/transports/smtp.spec.js b/test/components/emailer/transports/smtp.spec.js
index f2ee19e..c6a2e1a 100644
--- a/test/components/emailer/transports/smtp.spec.js
+++ b/test/components/emailer/transports/smtp.spec.js
@@ -10,7 +10,7 @@ describe('transports/smtp', () => {
     nodemailerSmtpTransport = sinon.stub();
 
     smtpTransport = proxyquire('../../../../components/emailer/transports/smtp', {
-      'nodemailer-smtp-transport': nodemailerSmtpTransport
+      nodemailer: nodemailerSmtpTransport
     });
   });

Would be good to get a confirmation if the above is a correct theory before I raise a PR if possible please?

Thanks

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

1 participant