Taint cookies and headers (#3232)

* Taint cookies and headers

* Bump minimum node version for v4 on cookie plugin test

* Add test with latest node version for cookie plugin test

* Provide iastContext from index when tainting headers

* Add test for cookie tainting in taint tracking plugin

* Remove iast transaction after taint tracking plugin tests to avoid hiting setMaxTransactions in tests

* Add test for taintObject with taintingKeys flag

* Address header tainting test for keys shorter than 10 chars

* Upgrade native-iast-taint-tracking to v1.5.0

* Rewrite expect in taint tracking plugin test

* Fix tag requiring in IAST index

Author: CarlesDD

.github/workflows/appsec.yml

@@ -136,3 +136,18 @@ jobs:
       - uses: ./.github/actions/node/latest
       - run: yarn test:appsec:plugins:ci
       - uses: codecov/codecov-action@v2
+
+  sourcing:
+    runs-on: ubuntu-latest
+    env:
+      PLUGINS: cookie
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ./.github/actions/node/setup
+      - run: yarn install
+      - uses: ./.github/actions/node/16
+      - run: yarn test:appsec:plugins:ci
+      - uses: ./.github/actions/node/18
+      - run: yarn test:appsec:plugins:ci
+      - uses: ./.github/actions/node/latest
+      - run: yarn test:appsec:plugins:ci

package.json

@@ -68,7 +68,7 @@
   "dependencies": {
     "@datadog/native-appsec": "^3.2.0",
     "@datadog/native-iast-rewriter": "2.0.1",
-    "@datadog/native-iast-taint-tracking": "^1.4.1",
+    "@datadog/native-iast-taint-tracking": "^1.5.0",
     "@datadog/native-metrics": "^1.6.0",
     "@datadog/pprof": "^2.2.1",
     "@datadog/sketches-js": "^2.1.0",

packages/datadog-instrumentations/src/cookie.js

@@ -0,0 +1,21 @@
+'use strict'
+
+const shimmer = require('../../datadog-shimmer')
+const { channel, addHook } = require('./helpers/instrument')
+
+const cookieParseCh = channel('datadog:cookie:parse:finish')
+
+function wrapParse (originalParse) {
+  return function () {
+    const cookies = originalParse.apply(this, arguments)
+    if (cookieParseCh.hasSubscribers && cookies) {
+      cookieParseCh.publish({ cookies })
+    }
+    return cookies
+  }
+}
+
+addHook({ name: 'cookie', versions: ['>=0.4'] }, cookie => {
+  shimmer.wrap(cookie, 'parse', wrapParse)
+  return cookie
+})

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

@@ -25,6 +25,7 @@ module.exports = {
   'child_process': () => require('../child-process'),
   'node:child_process': () => require('../child-process'),
   'connect': () => require('../connect'),
+  'cookie': () => require('../cookie'),
   'couchbase': () => require('../couchbase'),
   'crypto': () => require('../crypto'),
   'cypress': () => require('../cypress'),

packages/dd-trace/src/appsec/iast/index.js

@@ -5,7 +5,13 @@ const { storage } = require('../../../../datadog-core')
 const overheadController = require('./overhead-controller')
 const dc = require('../../../../diagnostics_channel')
 const iastContextFunctions = require('./iast-context')
-const { enableTaintTracking, disableTaintTracking, createTransaction, removeTransaction } = require('./taint-tracking')
+const {
+  enableTaintTracking,
+  disableTaintTracking,
+  createTransaction,
+  removeTransaction,
+  taintTrackingPlugin
+} = require('./taint-tracking')
 const { IAST_ENABLED_TAG_KEY } = require('./tags')
 
 const telemetryLogs = require('./telemetry/logs')
@@ -48,6 +54,7 @@ function onIncomingHttpRequestStart (data) {
           const iastContext = iastContextFunctions.saveIastContext(store, topContext, { rootSpan, req: data.req })
           createTransaction(rootSpan.context().toSpanId(), iastContext)
           overheadController.initializeRequestContext(iastContext)
+          taintTrackingPlugin.taintHeaders(data.req.headers, iastContext)
         }
         if (rootSpan.addTags) {
           rootSpan.addTags({

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

@@ -23,5 +23,6 @@ module.exports = {
   },
   setMaxTransactions: setMaxTransactions,
   createTransaction: createTransaction,
-  removeTransaction: removeTransaction
+  removeTransaction: removeTransaction,
+  taintTrackingPlugin
 }

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

@@ -30,14 +30,14 @@ function newTaintedString (iastContext, string, name, type) {
   return result
 }
 
-function taintObject (iastContext, object, type) {
+function taintObject (iastContext, object, type, keyTainting, keyType) {
   let result = object
   if (iastContext && iastContext[IAST_TRANSACTION_ID]) {
     const transactionId = iastContext[IAST_TRANSACTION_ID]
     const queue = [{ parent: null, property: null, value: object }]
     const visited = new WeakSet()
     while (queue.length > 0) {
-      const { parent, property, value } = queue.pop()
+      const { parent, property, value, key } = queue.pop()
       if (value === null) {
         continue
       }
@@ -47,14 +47,23 @@ function taintObject (iastContext, object, type) {
           if (!parent) {
             result = tainted
           } else {
-            parent[property] = tainted
+            if (keyTainting && key) {
+              const taintedProperty = TaintedUtils.newTaintedString(transactionId, key, property, keyType)
+              parent[taintedProperty] = tainted
+            } else {
+              parent[property] = tainted
+            }
           }
         } else if (typeof value === 'object' && !visited.has(value)) {
           visited.add(value)
           const keys = Object.keys(value)
           for (let i = 0; i < keys.length; i++) {
             const key = keys[i]
-            queue.push({ parent: value, property: property ? `${property}.${key}` : key, value: value[key] })
+            queue.push({ parent: value, property: property ? `${property}.${key}` : key, value: value[key], key })
+          }
+          if (parent && keyTainting && key) {
+            const taintedProperty = TaintedUtils.newTaintedString(transactionId, key, property, keyType)
+            parent[taintedProperty] = value
           }
         }
       } catch (e) {

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

@@ -3,5 +3,8 @@
 module.exports = {
   HTTP_REQUEST_BODY: 'http.request.body',
   HTTP_REQUEST_PARAMETER: 'http.request.parameter',
+  HTTP_REQUEST_COOKIE_VALUE: 'http.request.cookie.value',
+  HTTP_REQUEST_COOKIE_NAME: 'http.request.cookie.name',
+  HTTP_REQUEST_HEADER_NAME: 'http.request.header.name',
   HTTP_REQUEST_HEADER_VALUE: 'http.request.header'
 }

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

@@ -3,8 +3,15 @@
 const Plugin = require('../../../plugins/plugin')
 const { getIastContext } = require('../iast-context')
 const { storage } = require('../../../../../datadog-core')
-const { HTTP_REQUEST_PARAMETER, HTTP_REQUEST_BODY } = require('./origin-types')
 const { taintObject } = require('./operations')
+const {
+  HTTP_REQUEST_PARAMETER,
+  HTTP_REQUEST_BODY,
+  HTTP_REQUEST_COOKIE_VALUE,
+  HTTP_REQUEST_COOKIE_NAME,
+  HTTP_REQUEST_HEADER_VALUE,
+  HTTP_REQUEST_HEADER_NAME
+} = require('./origin-types')
 
 class TaintTrackingPlugin extends Plugin {
   constructor () {
@@ -22,8 +29,8 @@ class TaintTrackingPlugin extends Plugin {
     )
     this.addSub(
       'datadog:qs:parse:finish',
-      ({ qs }) => this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, qs))
-
+      ({ qs }) => this._taintTrackingHandler(HTTP_REQUEST_PARAMETER, qs)
+    )
     this.addSub('apm:express:middleware:next', ({ req }) => {
       if (req && req.body && typeof req.body === 'object') {
         const iastContext = getIastContext(storage.getStore())
@@ -33,16 +40,29 @@ class TaintTrackingPlugin extends Plugin {
         }
       }
     })
+    this.addSub(
+      'datadog:cookie:parse:finish',
+      ({ cookies }) => this._cookiesTaintTrackingHandler(cookies)
+    )
   }
 
   _taintTrackingHandler (type, target, property, iastContext = getIastContext(storage.getStore())) {
     if (!property) {
       taintObject(iastContext, target, type)
-    } else {
+    } else if (target[property]) {
       target[property] = taintObject(iastContext, target[property], type)
     }
   }
 
+  _cookiesTaintTrackingHandler (target) {
+    const iastContext = getIastContext(storage.getStore())
+    taintObject(iastContext, target, HTTP_REQUEST_COOKIE_VALUE, true, HTTP_REQUEST_COOKIE_NAME)
+  }
+
+  taintHeaders (headers, iastContext) {
+    taintObject(iastContext, headers, HTTP_REQUEST_HEADER_VALUE, true, HTTP_REQUEST_HEADER_NAME)
+  }
+
   enable () {
     this.configure(true)
   }

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

@@ -4,10 +4,15 @@ const proxyquire = require('proxyquire')
 const iastContextFunctions = require('../../../../src/appsec/iast/iast-context')
 const taintTrackingOperations = require('../../../../src/appsec/iast/taint-tracking/operations')
 const dc = require('../../../../../diagnostics_channel')
+const {
+  HTTP_REQUEST_COOKIE_VALUE,
+  HTTP_REQUEST_COOKIE_NAME
+} = require('../../../../src/appsec/iast/taint-tracking/origin-types')
 
 const middlewareNextChannel = dc.channel('apm:express:middleware:next')
 const queryParseFinishChannel = dc.channel('datadog:qs:parse:finish')
 const bodyParserFinishChannel = dc.channel('datadog:body-parser:read:finish')
+const cookieParseFinishCh = dc.channel('datadog:cookie:parse:finish')
 
 describe('IAST Taint tracking plugin', () => {
   let taintTrackingPlugin
@@ -33,11 +38,12 @@ describe('IAST Taint tracking plugin', () => {
     sinon.restore()
   })
 
-  it('Should subscribe to body parser and qs channel', () => {
-    expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(3)
+  it('Should subscribe to body parser, qs and cookie channel', () => {
+    expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(4)
     expect(taintTrackingPlugin._subscriptions[0]._channel.name).to.equals('datadog:body-parser:read:finish')
     expect(taintTrackingPlugin._subscriptions[1]._channel.name).to.equals('datadog:qs:parse:finish')
     expect(taintTrackingPlugin._subscriptions[2]._channel.name).to.equals('apm:express:middleware:next')
+    expect(taintTrackingPlugin._subscriptions[3]._channel.name).to.equals('datadog:cookie:parse:finish')
   })
 
   describe('taint sources', () => {
@@ -55,6 +61,10 @@ describe('IAST Taint tracking plugin', () => {
       )
     })
 
+    afterEach(() => {
+      taintTrackingOperations.removeTransaction(iastContext)
+    })
+
     it('Should taint full object', () => {
       const originType = 'ORIGIN_TYPE'
       const objToBeTainted = {
@@ -108,11 +118,7 @@ describe('IAST Taint tracking plugin', () => {
       }
 
       taintTrackingPlugin._taintTrackingHandler(originType, objToBeTainted, propertyToBeTainted)
-      expect(taintTrackingOperations.taintObject).to.be.calledOnceWith(
-        iastContext,
-        objToBeTainted[propertyToBeTainted],
-        originType
-      )
+      expect(taintTrackingOperations.taintObject).not.to.be.called
     })
 
     it('Should taint request parameter when qs event is published', () => {
@@ -180,5 +186,21 @@ describe('IAST Taint tracking plugin', () => {
         'http.request.body'
       )
     })
+
+    it('Should taint cookies when cookie parser event is published', () => {
+      const cookies = {
+        cookie1: 'tainted_cookie'
+      }
+
+      cookieParseFinishCh.publish({ cookies })
+
+      expect(taintTrackingOperations.taintObject).to.be.calledOnceWith(
+        iastContext,
+        cookies,
+        HTTP_REQUEST_COOKIE_VALUE,
+        true,
+        HTTP_REQUEST_COOKIE_NAME
+      )
+    })
   })
 })