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

[robertdeniszczyc2]

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:

├─┬ nodemailer-smtp-transport@2.7.4
│ └─┬ smtp-connection@2.12.0
│   └─┬ httpntlm@1.6.1
│     └── underscore@1.7.0

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

[robertdeniszczyc2]

This was resolved by #497