Use static vulnerability hash source when the cookie name is too long

docs/test.ts

@@ -128,6 +128,7 @@ tracer.init({
   },
   iast: {
     enabled: true,
+    cookieFilterPattern: '.*',
     requestSampling: 50,
     maxConcurrentRequests: 4,
     maxContextOperations: 30,
@@ -143,6 +144,7 @@ tracer.init({
   experimental: {
     iast: {
       enabled: true,
+      cookieFilterPattern: '.*',
       requestSampling: 50,
       maxConcurrentRequests: 4,
       maxContextOperations: 30,

index.d.ts

@@ -1739,7 +1739,6 @@ declare namespace tracer {
      * on the tracer.
      */
     interface pino extends Integration {}
-
     /**
      * This plugin automatically patches the [protobufjs](https://protobufjs.github.io/protobuf.js/)
      * to collect protobuf message schemas when Datastreams Monitoring is enabled.
@@ -2160,6 +2159,12 @@ declare namespace tracer {
      */
     maxContextOperations?: number,
 
+    /**
+     * Defines the pattern to ignore cookie names in the vulnerability hash calculation
+     * @default ".{32,}"
+     */
+    cookieFilterPattern?: string,
+
     /**
      * Whether to enable vulnerability deduplication
      */

packages/dd-trace/src/appsec/iast/analyzers/cookie-analyzer.js

@@ -2,6 +2,7 @@
 
 const Analyzer = require('./vulnerability-analyzer')
 const { getNodeModulesPaths } = require('../path-line')
+const iastLog = require('../iast-log')
 
 const EXCLUDED_PATHS = getNodeModulesPaths('express/lib/response.js')
 
@@ -11,7 +12,14 @@ class CookieAnalyzer extends Analyzer {
     this.propertyToBeSafe = propertyToBeSafe.toLowerCase()
   }
 
-  onConfigure () {
+  onConfigure (config) {
+    try {
+      this.cookieFilterRegExp = new RegExp(config.iast.cookieFilterPattern)
+    } catch {
+      iastLog.error('Invalid regex in cookieFilterPattern')
+      this.cookieFilterRegExp = /.{32,}/
+    }
+
     this.addSub(
       { channelName: 'datadog:iast:set-cookie', moduleName: 'http' },
       (cookieInfo) => this.analyze(cookieInfo)
@@ -28,6 +36,10 @@ class CookieAnalyzer extends Analyzer {
   }
 
   _createHashSource (type, evidence, location) {
+    if (typeof evidence.value === 'string' && evidence.value.match(this.cookieFilterRegExp)) {
+      return 'FILTERED_' + this._type
+    }
+
     return `${type}:${evidence.value}`
   }
 

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

@@ -127,7 +127,7 @@ class IastPlugin extends Plugin {
       config = { enabled: config }
     }
     if (config.enabled && !this.configured) {
-      this.onConfigure()
+      this.onConfigure(config.tracerConfig)
       this.configured = true
     }
 

packages/dd-trace/src/config.js

@@ -478,6 +478,7 @@ class Config {
     this._setValue(defaults, 'gitMetadataEnabled', true)
     this._setValue(defaults, 'headerTags', [])
     this._setValue(defaults, 'hostname', '127.0.0.1')
+    this._setValue(defaults, 'iast.cookieFilterPattern', '.{32,}')
     this._setValue(defaults, 'iast.deduplicationEnabled', true)
     this._setValue(defaults, 'iast.enabled', false)
     this._setValue(defaults, 'iast.maxConcurrentRequests', 2)
@@ -581,6 +582,7 @@ class Config {
       DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED,
       DD_EXPERIMENTAL_PROFILING_ENABLED,
       JEST_WORKER_ID,
+      DD_IAST_COOKIE_FILTER_PATTERN,
       DD_IAST_DEDUPLICATION_ENABLED,
       DD_IAST_ENABLED,
       DD_IAST_MAX_CONCURRENT_REQUESTS,
@@ -716,6 +718,7 @@ class Config {
     this._setBoolean(env, 'gitMetadataEnabled', DD_TRACE_GIT_METADATA_ENABLED)
     this._setArray(env, 'headerTags', DD_TRACE_HEADER_TAGS)
     this._setString(env, 'hostname', coalesce(DD_AGENT_HOST, DD_TRACE_AGENT_HOSTNAME))
+    this._setString(env, 'iast.cookieFilterPattern', DD_IAST_COOKIE_FILTER_PATTERN)
     this._setBoolean(env, 'iast.deduplicationEnabled', DD_IAST_DEDUPLICATION_ENABLED)
     this._setBoolean(env, 'iast.enabled', DD_IAST_ENABLED)
     this._setValue(env, 'iast.maxConcurrentRequests', maybeInt(DD_IAST_MAX_CONCURRENT_REQUESTS))
@@ -884,6 +887,7 @@ class Config {
     this._optsUnprocessed.flushMinSpans = options.flushMinSpans
     this._setArray(opts, 'headerTags', options.headerTags)
     this._setString(opts, 'hostname', options.hostname)
+    this._setString(opts, 'iast.cookieFilterPattern', options.iast?.cookieFilterPattern)
     this._setBoolean(opts, 'iast.deduplicationEnabled', options.iast && options.iast.deduplicationEnabled)
     this._setBoolean(opts, 'iast.enabled',
       options.iast && (options.iast === true || options.iast.enabled === true))

packages/dd-trace/test/appsec/iast/analyzers/cookie-analyzer.spec.js

@@ -0,0 +1,110 @@
+'use strict'
+
+const { assert } = require('chai')
+const CookieAnalyzer = require('../../../../src/appsec/iast/analyzers/cookie-analyzer')
+const Analyzer = require('../../../../src/appsec/iast/analyzers/vulnerability-analyzer')
+const Config = require('../../../../src/config')
+
+describe('CookieAnalyzer', () => {
+  const VULNERABILITY_TYPE = 'VULN_TYPE'
+
+  it('should extends Analyzer', () => {
+    assert.isTrue(Analyzer.isPrototypeOf(CookieAnalyzer))
+  })
+
+  describe('_createHashSource', () => {
+    let cookieAnalyzer
+
+    beforeEach(() => {
+      cookieAnalyzer = new CookieAnalyzer(VULNERABILITY_TYPE, 'prop')
+    })
+
+    describe('default config', () => {
+      beforeEach(() => {
+        cookieAnalyzer.onConfigure(new Config({ iast: true }))
+      })
+
+      it('should create hash from vulnerability type and not long enough evidence value', () => {
+        const evidence = {
+          value: '0'.repeat(31)
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`${VULNERABILITY_TYPE}:${evidence.value}`))
+      })
+
+      it('should create different hash from vulnerability type and long evidence value', () => {
+        const evidence = {
+          value: '0'.repeat(32)
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`FILTERED_${VULNERABILITY_TYPE}`))
+      })
+    })
+
+    describe('custom cookieFilterPattern', () => {
+      beforeEach(() => {
+        cookieAnalyzer.onConfigure(new Config({
+          iast: {
+            enabled: true,
+            cookieFilterPattern: '^filtered$'
+          }
+        }))
+      })
+
+      it('should create hash from vulnerability with the default pattern', () => {
+        const evidence = {
+          value: 'notfiltered'
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`${VULNERABILITY_TYPE}:${evidence.value}`))
+      })
+
+      it('should create different hash from vulnerability type and long evidence value', () => {
+        const evidence = {
+          value: 'filtered'
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`FILTERED_${VULNERABILITY_TYPE}`))
+      })
+    })
+
+    describe('invalid cookieFilterPattern maintains default behaviour', () => {
+      beforeEach(() => {
+        cookieAnalyzer.onConfigure(new Config({
+          iast: {
+            enabled: true,
+            cookieFilterPattern: '('
+          }
+        }))
+      })
+
+      it('should create hash from vulnerability type and not long enough evidence value', () => {
+        const evidence = {
+          value: '0'.repeat(31)
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`${VULNERABILITY_TYPE}:${evidence.value}`))
+      })
+
+      it('should create different hash from vulnerability type and long evidence value', () => {
+        const evidence = {
+          value: '0'.repeat(32)
+        }
+
+        const vulnerability = cookieAnalyzer._createVulnerability(VULNERABILITY_TYPE, evidence, null, {})
+
+        assert.equal(vulnerability.hash, cookieAnalyzer._createHash(`FILTERED_${VULNERABILITY_TYPE}`))
+      })
+    })
+  })
+})

packages/dd-trace/test/appsec/iast/analyzers/insecure-cookie-analyzer.spec.js

@@ -3,12 +3,20 @@
 const { prepareTestServerForIast } = require('../utils')
 const Analyzer = require('../../../../src/appsec/iast/analyzers/vulnerability-analyzer')
 const { INSECURE_COOKIE } = require('../../../../src/appsec/iast/vulnerabilities')
+const insecureCookieAnalyzer = require('../../../../src/appsec/iast/analyzers/insecure-cookie-analyzer')
+const CookieAnalyzer = require('../../../../src/appsec/iast/analyzers/cookie-analyzer')
+
 const analyzer = new Analyzer()
 
 describe('insecure cookie analyzer', () => {
   it('Expected vulnerability identifier', () => {
     expect(INSECURE_COOKIE).to.be.equals('INSECURE_COOKIE')
   })
+
+  it('InsecureCookieAnalyzer extends CookieAnalyzer', () => {
+    expect(CookieAnalyzer.isPrototypeOf(insecureCookieAnalyzer.constructor)).to.be.true
+  })
+
   // In these test, even when we are having multiple vulnerabilities, all the vulnerabilities
   // are in the same cookies method, and it is expected to detect both even when the max operations is 1
   const iastConfig = {
@@ -43,6 +51,12 @@ describe('insecure cookie analyzer', () => {
         res.setHeader('set-cookie', ['key=value; HttpOnly', 'key2=value2; Secure'])
       }, INSECURE_COOKIE, 1)
 
+      testThatRequestHasVulnerability((req, res) => {
+        const cookieNamePrefix = '0'.repeat(32)
+        res.setHeader('set-cookie', [cookieNamePrefix + 'key1=value', cookieNamePrefix + 'key2=value2'])
+      }, INSECURE_COOKIE, 1, undefined, undefined,
+      'Should be detected as the same INSECURE_COOKIE vulnerability when the cookie name is long')
+
       testThatRequestHasNoVulnerability((req, res) => {
         res.setHeader('set-cookie', 'key=value; Secure')
       }, INSECURE_COOKIE)

packages/dd-trace/test/appsec/iast/analyzers/no-httponly-cookie-analyzer.spec.js

@@ -3,13 +3,20 @@
 const { prepareTestServerForIast } = require('../utils')
 const Analyzer = require('../../../../src/appsec/iast/analyzers/vulnerability-analyzer')
 const { NO_HTTPONLY_COOKIE } = require('../../../../src/appsec/iast/vulnerabilities')
+const CookieAnalyzer = require('../../../../src/appsec/iast/analyzers/cookie-analyzer')
+const noHttponlyCookieAnalyzer = require('../../../../src/appsec/iast/analyzers/no-httponly-cookie-analyzer')
+
 const analyzer = new Analyzer()
 
 describe('no HttpOnly cookie analyzer', () => {
   it('Expected vulnerability identifier', () => {
     expect(NO_HTTPONLY_COOKIE).to.be.equals('NO_HTTPONLY_COOKIE')
   })
 
+  it('NoHttponlyCookieAnalyzer extends CookieAnalyzer', () => {
+    expect(CookieAnalyzer.isPrototypeOf(noHttponlyCookieAnalyzer.constructor)).to.be.true
+  })
+
   // In these test, even when we are having multiple vulnerabilities, all the vulnerabilities
   // are in the same cookies method, and it is expected to detect both even when the max operations is 1
   const iastConfig = {
@@ -18,6 +25,7 @@ describe('no HttpOnly cookie analyzer', () => {
     maxConcurrentRequests: 1,
     maxContextOperations: 1
   }
+
   prepareTestServerForIast('no HttpOnly cookie analyzer',
     (testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
       testThatRequestHasVulnerability((req, res) => {
@@ -47,6 +55,12 @@ describe('no HttpOnly cookie analyzer', () => {
         res.setHeader('set-cookie', ['key=value; HttpOnly', 'key2=value2; Secure'])
       }, NO_HTTPONLY_COOKIE, 1)
 
+      testThatRequestHasVulnerability((req, res) => {
+        const cookieNamePrefix = '0'.repeat(32)
+        res.setHeader('set-cookie', [cookieNamePrefix + 'key1=value', cookieNamePrefix + 'key2=value2'])
+      }, NO_HTTPONLY_COOKIE, 1, undefined, undefined,
+      'Should be detected as the same NO_HTTPONLY_COOKIE vulnerability when the cookie name is long')
+
       testThatRequestHasNoVulnerability((req, res) => {
         res.setHeader('set-cookie', 'key=value; HttpOnly')
       }, NO_HTTPONLY_COOKIE)

packages/dd-trace/test/appsec/iast/analyzers/no-samesite-cookie-analyzer.spec.js

@@ -3,13 +3,20 @@
 const { prepareTestServerForIast } = require('../utils')
 const Analyzer = require('../../../../src/appsec/iast/analyzers/vulnerability-analyzer')
 const { NO_SAMESITE_COOKIE } = require('../../../../src/appsec/iast/vulnerabilities')
+const CookieAnalyzer = require('../../../../src/appsec/iast/analyzers/cookie-analyzer')
+const noSamesiteCookieAnalyzer = require('../../../../src/appsec/iast/analyzers/no-samesite-cookie-analyzer')
+
 const analyzer = new Analyzer()
 
 describe('no SameSite cookie analyzer', () => {
   it('Expected vulnerability identifier', () => {
     expect(NO_SAMESITE_COOKIE).to.be.equals('NO_SAMESITE_COOKIE')
   })
 
+  it('NoSamesiteCookieAnalyzer extends CookieAnalyzer', () => {
+    expect(CookieAnalyzer.isPrototypeOf(noSamesiteCookieAnalyzer.constructor)).to.be.true
+  })
+
   // In these test, even when we are having multiple vulnerabilities, all the vulnerabilities
   // are in the same cookies method, and it is expected to detect both even when the max operations is 1
   const iastConfig = {
@@ -59,6 +66,12 @@ describe('no SameSite cookie analyzer', () => {
         res.setHeader('set-cookie', 'key=value; SameSite=strict')
       }, NO_SAMESITE_COOKIE)
 
+      testThatRequestHasVulnerability((req, res) => {
+        const cookieNamePrefix = '0'.repeat(32)
+        res.setHeader('set-cookie', [cookieNamePrefix + 'key1=value', cookieNamePrefix + 'key2=value2'])
+      }, NO_SAMESITE_COOKIE, 1, undefined, undefined,
+      'Should be detected as the same NO_SAMESITE_COOKIE vulnerability when the cookie name is long')
+
       testThatRequestHasNoVulnerability((req, res) => {
         res.setHeader('set-cookie', 'key=')
       }, NO_SAMESITE_COOKIE)

packages/dd-trace/test/appsec/iast/utils.js

@@ -112,9 +112,7 @@ function beforeEachIastTest (iastConfig) {
   beforeEach(() => {
     vulnerabilityReporter.clearCache()
     iast.enable(new Config({
-      experimental: {
-        iast: iastConfig
-      }
+      iast: iastConfig
     }))
   })
 }
@@ -249,8 +247,8 @@ function prepareTestServerForIast (description, tests, iastConfig) {
       return agent.close({ ritmReset: false })
     })
 
-    function testThatRequestHasVulnerability (fn, vulnerability, occurrences, cb, makeRequest) {
-      it(`should have ${vulnerability} vulnerability`, function (done) {
+    function testThatRequestHasVulnerability (fn, vulnerability, occurrences, cb, makeRequest, description) {
+      it(description || `should have ${vulnerability} vulnerability`, function (done) {
         this.timeout(5000)
         app = fn
         checkVulnerabilityInRequest(vulnerability, occurrences, cb, makeRequest, config, done)