Skip to content

Commit

Permalink
Filter addresses before calling to waf to prevent running without kno…
Browse files Browse the repository at this point in the history
…wnAddresses (#4656)
  • Loading branch information
uurien authored and juan-fernandez committed Oct 1, 2024
1 parent 9a86318 commit f05454a
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 18 deletions.
37 changes: 23 additions & 14 deletions packages/dd-trace/src/appsec/waf/waf_context_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ const preventDuplicateAddresses = new Set([
])

class WAFContextWrapper {
constructor (ddwafContext, wafTimeout, wafVersion, rulesVersion) {
constructor (ddwafContext, wafTimeout, wafVersion, rulesVersion, knownAddresses) {
this.ddwafContext = ddwafContext
this.wafTimeout = wafTimeout
this.wafVersion = wafVersion
this.rulesVersion = rulesVersion
this.addressesToSkip = new Set()
this.knownAddresses = knownAddresses
}

run ({ persistent, ephemeral }, raspRuleType) {
Expand All @@ -27,31 +28,39 @@ class WAFContextWrapper {

const payload = {}
let payloadHasData = false
const inputs = {}
const newAddressesToSkip = new Set(this.addressesToSkip)

if (persistent !== null && typeof persistent === 'object') {
// TODO: possible optimization: only send params that haven't already been sent with same value to this wafContext
const persistentInputs = {}

for (const key of Object.keys(persistent)) {
// TODO: requiredAddresses is no longer used due to processor addresses are not included in the list. Check on
// future versions when the actual addresses are included in the 'loaded' section inside diagnostics.
if (!this.addressesToSkip.has(key)) {
inputs[key] = persistent[key]
if (!this.addressesToSkip.has(key) && this.knownAddresses.has(key)) {
persistentInputs[key] = persistent[key]
if (preventDuplicateAddresses.has(key)) {
newAddressesToSkip.add(key)
}
}
}
}

if (Object.keys(inputs).length) {
payload.persistent = inputs
payloadHasData = true
if (Object.keys(persistentInputs).length) {
payload.persistent = persistentInputs
payloadHasData = true
}
}

if (ephemeral && Object.keys(ephemeral).length) {
payload.ephemeral = ephemeral
payloadHasData = true
if (ephemeral !== null && typeof ephemeral === 'object') {
const ephemeralInputs = {}

for (const key of Object.keys(ephemeral)) {
if (this.knownAddresses.has(key)) {
ephemeralInputs[key] = ephemeral[key]
}
}

if (Object.keys(ephemeralInputs).length) {
payload.ephemeral = ephemeralInputs
payloadHasData = true
}
}

if (!payloadHasData) return
Expand Down
3 changes: 2 additions & 1 deletion packages/dd-trace/src/appsec/waf/waf_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class WAFManager {
this.ddwaf.createContext(),
this.wafTimeout,
this.ddwafVersion,
this.rulesVersion
this.rulesVersion,
this.ddwaf.knownAddresses
)
contexts.set(req, wafContext)
}
Expand Down
9 changes: 9 additions & 0 deletions packages/dd-trace/test/appsec/waf/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ const Reporter = require('../../../src/appsec/reporter')
const web = require('../../../src/plugins/util/web')

describe('WAF Manager', () => {
const knownAddresses = new Set([
'server.io.net.url',
'server.request.headers.no_cookies',
'server.request.uri.raw',
'processor.address',
'server.request.body',
'waf.context.processor'
])
let waf, WAFManager
let DDWAF
let config
Expand All @@ -26,6 +34,7 @@ describe('WAF Manager', () => {
loaded: ['rule_1'], failed: []
}
}
DDWAF.prototype.knownAddresses = knownAddresses

WAFManager = proxyquire('../../../src/appsec/waf/waf_manager', {
'@datadog/native-appsec': { DDWAF }
Expand Down
31 changes: 28 additions & 3 deletions packages/dd-trace/test/appsec/waf/waf_context_wrapper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ const WAFContextWrapper = require('../../../src/appsec/waf/waf_context_wrapper')
const addresses = require('../../../src/appsec/addresses')

describe('WAFContextWrapper', () => {
const knownAddresses = new Set([
addresses.HTTP_INCOMING_QUERY,
addresses.HTTP_INCOMING_GRAPHQL_RESOLVER
])

it('Should send HTTP_INCOMING_QUERY only once', () => {
const ddwafContext = {
run: sinon.stub()
}
const wafContextWrapper = new WAFContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0')
const wafContextWrapper = new WAFContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0', knownAddresses)

const payload = {
persistent: {
Expand All @@ -27,7 +32,7 @@ describe('WAFContextWrapper', () => {
const ddwafContext = {
run: sinon.stub()
}
const wafContextWrapper = new WAFContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0')
const wafContextWrapper = new WAFContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0', knownAddresses)

const payload = {
persistent: {
Expand All @@ -52,6 +57,26 @@ describe('WAFContextWrapper', () => {
}, 1000)
})

it('Should ignore run without known addresses', () => {
const ddwafContext = {
run: sinon.stub()
}
const wafContextWrapper = new WAFContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0', knownAddresses)

const payload = {
persistent: {
'persistent-unknown-address': { key: 'value' }
},
ephemeral: {
'ephemeral-unknown-address': { key: 'value' }
}
}

wafContextWrapper.run(payload)

expect(ddwafContext.run).to.have.not.been.called
})

describe('Disposal context check', () => {
let log
let ddwafContext
Expand All @@ -70,7 +95,7 @@ describe('WAFContextWrapper', () => {
'../../log': log
})

wafContextWrapper = new ProxiedWafContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0')
wafContextWrapper = new ProxiedWafContextWrapper(ddwafContext, 1000, '1.14.0', '1.8.0', knownAddresses)
})

afterEach(() => {
Expand Down
33 changes: 33 additions & 0 deletions packages/dd-trace/test/appsec/waf/waf_manager.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict'

const proxyquire = require('proxyquire')

describe('WAFManager', () => {
let WAFManager, WAFContextWrapper, DDWAF
const knownAddresses = new Set()

beforeEach(() => {
DDWAF = sinon.stub()
DDWAF.prototype.constructor.version = sinon.stub()
DDWAF.prototype.knownAddresses = knownAddresses
DDWAF.prototype.diagnostics = {}
DDWAF.prototype.createContext = sinon.stub()

WAFContextWrapper = sinon.stub()
WAFManager = proxyquire('../../../src/appsec/waf/waf_manager', {
'./waf_context_wrapper': WAFContextWrapper,
'@datadog/native-appsec': { DDWAF }
})
})

describe('getWAFContext', () => {
it('should construct WAFContextWrapper with knownAddresses', () => {
const wafManager = new WAFManager({}, {})

wafManager.getWAFContext({})

const any = sinon.match.any
sinon.assert.calledOnceWithMatch(WAFContextWrapper, any, any, any, any, knownAddresses)
})
})
})

0 comments on commit f05454a

Please sign in to comment.