Revert "Taint cookies and headers (#3182)" (#3192)

This reverts commit cc51732.

.github/workflows/appsec.yml

@@ -136,18 +136,3 @@ 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

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

@@ -25,7 +25,6 @@ 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,13 +5,7 @@ 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,
-  taintTrackingPlugin
-} = require('./taint-tracking')
+const { enableTaintTracking, disableTaintTracking, createTransaction, removeTransaction } = require('./taint-tracking')
 
 const telemetryLogs = require('./telemetry/logs')
 const IAST_ENABLED_TAG_KEY = '_dd.iast.enabled'
@@ -54,7 +48,6 @@ 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,6 +23,5 @@ module.exports = {
   },
   setMaxTransactions: setMaxTransactions,
   createTransaction: createTransaction,
-  removeTransaction: removeTransaction,
-  taintTrackingPlugin
+  removeTransaction: removeTransaction
 }

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, keyTainting, keyType) {
+function taintObject (iastContext, object, type) {
   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, key } = queue.pop()
+      const { parent, property, value } = queue.pop()
       if (value === null) {
         continue
       }
@@ -47,23 +47,14 @@ function taintObject (iastContext, object, type, keyTainting, keyType) {
           if (!parent) {
             result = tainted
           } else {
-            if (keyTainting && key) {
-              const taintedProperty = TaintedUtils.newTaintedString(transactionId, key, property, keyType)
-              parent[taintedProperty] = tainted
-            } else {
-              parent[property] = tainted
-            }
+            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], key })
-          }
-          if (parent && keyTainting && key) {
-            const taintedProperty = TaintedUtils.newTaintedString(transactionId, key, property, keyType)
-            parent[taintedProperty] = value
+            queue.push({ parent: value, property: property ? `${property}.${key}` : key, value: value[key] })
           }
         }
       } catch (e) {

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

@@ -2,9 +2,5 @@
 
 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'
+  HTTP_REQUEST_PARAMETER: 'http.request.parameter'
 }

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

@@ -3,15 +3,8 @@
 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 () {
@@ -29,8 +22,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())
@@ -40,29 +33,16 @@ 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 if (target[property]) {
+    } else {
       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,15 +4,10 @@ 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
@@ -38,12 +33,11 @@ describe('IAST Taint tracking plugin', () => {
     sinon.restore()
   })
 
-  it('Should subscribe to body parser, qs and cookie channel', () => {
-    expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(4)
+  it('Should subscribe to body parser and qs channel', () => {
+    expect(taintTrackingPlugin._subscriptions).to.have.lengthOf(3)
     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', () => {
@@ -61,10 +55,6 @@ describe('IAST Taint tracking plugin', () => {
       )
     })
 
-    afterEach(() => {
-      taintTrackingOperations.removeTransaction(iastContext)
-    })
-
     it('Should taint full object', () => {
       const originType = 'ORIGIN_TYPE'
       const objToBeTainted = {
@@ -118,7 +108,11 @@ describe('IAST Taint tracking plugin', () => {
       }
 
       taintTrackingPlugin._taintTrackingHandler(originType, objToBeTainted, propertyToBeTainted)
-      expect(taintTrackingOperations.taintObject).to.not.be.called
+      expect(taintTrackingOperations.taintObject).to.be.calledOnceWith(
+        iastContext,
+        objToBeTainted[propertyToBeTainted],
+        originType
+      )
     })
 
     it('Should taint request parameter when qs event is published', () => {
@@ -186,21 +180,5 @@ 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
-      )
-    })
   })
 })