Support url.parse, url.URL.parse and new url.URL for IAST taint track…

…ing (#4836)

* Support url.parse, url.URL.parse and new url.URL for taint tracking

* Address PR comments

* Use shimmer.wrap instead of doing it manually

.github/workflows/plugins.yml

@@ -578,7 +578,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: ./.github/actions/plugins/test
-  
+
   mariadb:
     runs-on: ubuntu-latest
     services:
@@ -999,6 +999,14 @@ jobs:
       - uses: actions/checkout@v4
       - uses: ./.github/actions/plugins/test
 
+  url:
+    runs-on: ubuntu-latest
+    env:
+      PLUGINS: url
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/plugins/test
+
   when:
     runs-on: ubuntu-latest
     env:

packages/datadog-instrumentations/src/helpers/hooks.js

@@ -91,6 +91,7 @@ module.exports = {
   'node:http2': () => require('../http2'),
   'node:https': () => require('../http'),
   'node:net': () => require('../net'),
+  'node:url': () => require('../url'),
   nyc: () => require('../nyc'),
   oracledb: () => require('../oracledb'),
   openai: () => require('../openai'),
@@ -115,6 +116,7 @@ module.exports = {
   sharedb: () => require('../sharedb'),
   tedious: () => require('../tedious'),
   undici: () => require('../undici'),
+  url: () => require('../url'),
   vitest: { esmFirst: true, fn: () => require('../vitest') },
   when: () => require('../when'),
   winston: () => require('../winston'),

packages/datadog-instrumentations/src/url.js

@@ -0,0 +1,84 @@
+'use strict'
+
+const { addHook, channel } = require('./helpers/instrument')
+const shimmer = require('../../datadog-shimmer')
+const names = ['url', 'node:url']
+
+const parseFinishedChannel = channel('datadog:url:parse:finish')
+const urlGetterChannel = channel('datadog:url:getter:finish')
+const instrumentedGetters = ['host', 'origin', 'hostname']
+
+addHook({ name: names }, function (url) {
+  shimmer.wrap(url, 'parse', (parse) => {
+    return function wrappedParse (input) {
+      const parsedValue = parse.apply(this, arguments)
+      if (!parseFinishedChannel.hasSubscribers) return parsedValue
+
+      parseFinishedChannel.publish({
+        input,
+        parsed: parsedValue,
+        isURL: false
+      })
+
+      return parsedValue
+    }
+  })
+
+  const URLPrototype = url.URL.prototype.constructor.prototype
+  instrumentedGetters.forEach(property => {
+    const originalDescriptor = Object.getOwnPropertyDescriptor(URLPrototype, property)
+
+    if (originalDescriptor?.get) {
+      const newDescriptor = shimmer.wrap(originalDescriptor, 'get', function (originalGet) {
+        return function get () {
+          const result = originalGet.apply(this, arguments)
+          if (!urlGetterChannel.hasSubscribers) return result
+
+          const context = { urlObject: this, result, property }
+          urlGetterChannel.publish(context)
+
+          return context.result
+        }
+      })
+
+      Object.defineProperty(URLPrototype, property, newDescriptor)
+    }
+  })
+
+  shimmer.wrap(url, 'URL', (URL) => {
+    return class extends URL {
+      constructor (input, base) {
+        super(...arguments)
+
+        if (!parseFinishedChannel.hasSubscribers) return
+
+        parseFinishedChannel.publish({
+          input,
+          base,
+          parsed: this,
+          isURL: true
+        })
+      }
+    }
+  })
+
+  if (url.URL.parse) {
+    shimmer.wrap(url.URL, 'parse', (parse) => {
+      return function wrappedParse (input, base) {
+        const parsedValue = parse.apply(this, arguments)
+        if (!parseFinishedChannel.hasSubscribers) return parsedValue
+
+        parseFinishedChannel.publish({
+          input,
+          base,
+          parsed: parsedValue,
+          isURL: true
+        })
+
+        return parsedValue
+      }
+    })
+  }
+
+  return url
+})

packages/datadog-instrumentations/test/url.spec.js

@@ -0,0 +1,114 @@
+'use strict'
+
+const agent = require('../../dd-trace/test/plugins/agent')
+const { channel } = require('../src/helpers/instrument')
+const names = ['url', 'node:url']
+
+names.forEach(name => {
+  describe(name, () => {
+    const url = require(name)
+    const parseFinishedChannel = channel('datadog:url:parse:finish')
+    const urlGetterChannel = channel('datadog:url:getter:finish')
+    let parseFinishedChannelCb, urlGetterChannelCb
+
+    before(async () => {
+      await agent.load('url')
+    })
+
+    after(() => {
+      return agent.close()
+    })
+
+    beforeEach(() => {
+      parseFinishedChannelCb = sinon.stub()
+      urlGetterChannelCb = sinon.stub()
+      parseFinishedChannel.subscribe(parseFinishedChannelCb)
+      urlGetterChannel.subscribe(urlGetterChannelCb)
+    })
+
+    afterEach(() => {
+      parseFinishedChannel.unsubscribe(parseFinishedChannelCb)
+      urlGetterChannel.unsubscribe(urlGetterChannelCb)
+    })
+
+    describe('url.parse', () => {
+      it('should publish', () => {
+        // eslint-disable-next-line n/no-deprecated-api
+        const result = url.parse('https://www.datadoghq.com')
+
+        sinon.assert.calledOnceWithExactly(parseFinishedChannelCb, {
+          input: 'https://www.datadoghq.com',
+          parsed: result,
+          isURL: false
+        }, sinon.match.any)
+      })
+    })
+
+    describe('url.URL', () => {
+      describe('new URL', () => {
+        it('should publish with input', () => {
+          const result = new url.URL('https://www.datadoghq.com')
+
+          sinon.assert.calledOnceWithExactly(parseFinishedChannelCb, {
+            input: 'https://www.datadoghq.com',
+            base: undefined,
+            parsed: result,
+            isURL: true
+          }, sinon.match.any)
+        })
+
+        it('should publish with base and input', () => {
+          const result = new url.URL('/path', 'https://www.datadoghq.com')
+
+          sinon.assert.calledOnceWithExactly(parseFinishedChannelCb, {
+            base: 'https://www.datadoghq.com',
+            input: '/path',
+            parsed: result,
+            isURL: true
+          }, sinon.match.any)
+        })
+
+        ;['host', 'origin', 'hostname'].forEach(property => {
+          it(`should publish on get ${property}`, () => {
+            const urlObject = new url.URL('/path', 'https://www.datadoghq.com')
+
+            const result = urlObject[property]
+
+            sinon.assert.calledWithExactly(urlGetterChannelCb, {
+              urlObject,
+              result,
+              property
+            }, sinon.match.any)
+          })
+        })
+      })
+    })
+
+    if (url.URL.parse) { // added in v22.1.0
+      describe('url.URL.parse', () => {
+        it('should publish with input', () => {
+          const input = 'https://www.datadoghq.com'
+          const parsed = url.URL.parse(input)
+
+          sinon.assert.calledOnceWithExactly(parseFinishedChannelCb, {
+            input,
+            parsed,
+            base: undefined,
+            isURL: true
+          }, sinon.match.any)
+        })
+
+        it('should publish with base and input', () => {
+          const result = new url.URL('/path', 'https://www.datadoghq.com')
+
+          sinon.assert.calledOnceWithExactly(parseFinishedChannelCb, {
+            base: 'https://www.datadoghq.com',
+            input: '/path',
+            parsed: result,
+            isURL: true
+          }, sinon.match.any)
+        })
+      })
+    }
+  })
+})

packages/dd-trace/src/appsec/iast/taint-tracking/plugin.js

@@ -23,6 +23,7 @@ class TaintTrackingPlugin extends SourceIastPlugin {
   constructor () {
     super()
     this._type = 'taint-tracking'
+    this._taintedURLs = new WeakMap()
   }
 
   onConfigure () {
@@ -88,6 +89,46 @@ class TaintTrackingPlugin extends SourceIastPlugin {
       }
     )
 
+    const urlResultTaintedProperties = ['host', 'origin', 'hostname']
+    this.addSub(
+      { channelName: 'datadog:url:parse:finish' },
+      ({ input, base, parsed, isURL }) => {
+        const iastContext = getIastContext(storage.getStore())
+        let ranges
+
+        if (base) {
+          ranges = getRanges(iastContext, base)
+        } else {
+          ranges = getRanges(iastContext, input)
+        }
+
+        if (ranges?.length) {
+          if (isURL) {
+            this._taintedURLs.set(parsed, ranges[0])
+          } else {
+            urlResultTaintedProperties.forEach(param => {
+              this._taintTrackingHandler(ranges[0].iinfo.type, parsed, param, iastContext)
+            })
+          }
+        }
+      }
+    )
+
+    this.addSub(
+      { channelName: 'datadog:url:getter:finish' },
+      (context) => {
+        if (!urlResultTaintedProperties.includes(context.property)) return
+
+        const origRange = this._taintedURLs.get(context.urlObject)
+        if (!origRange) return
+
+        const iastContext = getIastContext(storage.getStore())
+        if (!iastContext) return
+
+        context.result =
+          newTaintedString(iastContext, context.result, origRange.iinfo.parameterName, origRange.iinfo.type)
+      })
+
     // this is a special case to increment INSTRUMENTED_SOURCE metric for header
     this.addInstrumentedSource('http', [HTTP_REQUEST_HEADER_VALUE, HTTP_REQUEST_HEADER_NAME])
   }

packages/dd-trace/test/appsec/iast/taint-tracking/plugin.express.plugin.spec.js

@@ -2,6 +2,7 @@
 
 const { prepareTestServerForIastInExpress } = require('../utils')
 const axios = require('axios')
+const { URL } = require('url')
 
 function noop () {}
 
@@ -47,6 +48,95 @@ describe('Taint tracking plugin sources express tests', () => {
             childProcess.exec(req.headers['x-iast-test-command'], noop)
           }, 'COMMAND_INJECTION', 1, noop, makeRequestWithHeader)
         })
+
+        describe('url parse taint tracking', () => {
+          function makePostRequest (done) {
+            axios.post(`http://localhost:${config.port}/`, {
+              url: 'http://www.datadoghq.com/'
+            }).catch(done)
+          }
+
+          testThatRequestHasVulnerability(
+            {
+              fn: (req) => {
+                // eslint-disable-next-line n/no-deprecated-api
+                const { parse } = require('url')
+                const url = parse(req.body.url)
+
+                const childProcess = require('child_process')
+                childProcess.exec(url.host, noop)
+              },
+              vulnerability: 'COMMAND_INJECTION',
+              occurrences: 1,
+              cb: noop,
+              makeRequest: makePostRequest,
+              testDescription: 'should detect vulnerability when tainted is coming from url.parse'
+            })
+
+          testThatRequestHasVulnerability(
+            {
+              fn: (req) => {
+                const { URL } = require('url')
+                const url = new URL(req.body.url)
+
+                const childProcess = require('child_process')
+                childProcess.exec(url.host, noop)
+              },
+              vulnerability: 'COMMAND_INJECTION',
+              occurrences: 1,
+              cb: noop,
+              makeRequest: makePostRequest,
+              testDescription: 'should detect vulnerability when tainted is coming from new url.URL input'
+            })
+
+          testThatRequestHasVulnerability(
+            {
+              fn: (req) => {
+                const { URL } = require('url')
+                const url = new URL('/path', req.body.url)
+
+                const childProcess = require('child_process')
+                childProcess.exec(url.host, noop)
+              },
+              vulnerability: 'COMMAND_INJECTION',
+              occurrences: 1,
+              cb: noop,
+              makeRequest: makePostRequest,
+              testDescription: 'should detect vulnerability when tainted is coming from new url.URL base'
+            })
+
+          if (URL.parse) {
+            testThatRequestHasVulnerability(
+              {
+                fn: (req) => {
+                  const { URL } = require('url')
+                  const url = URL.parse(req.body.url)
+                  const childProcess = require('child_process')
+                  childProcess.exec(url.host, noop)
+                },
+                vulnerability: 'COMMAND_INJECTION',
+                occurrences: 1,
+                cb: noop,
+                makeRequest: makePostRequest,
+                testDescription: 'should detect vulnerability when tainted is coming from url.URL.parse input'
+              })
+
+            testThatRequestHasVulnerability(
+              {
+                fn: (req) => {
+                  const { URL } = require('url')
+                  const url = URL.parse('/path', req.body.url)
+                  const childProcess = require('child_process')
+                  childProcess.exec(url.host, noop)
+                },
+                vulnerability: 'COMMAND_INJECTION',
+                occurrences: 1,
+                cb: noop,
+                makeRequest: makePostRequest,
+                testDescription: 'should detect vulnerability when tainted is coming from url.URL.parse base'
+              })
+          }
+        })
       }
     )
   })