Skip to content

Commit

Permalink
No SameSite cookie vulnerability detection (#3246)
Browse files Browse the repository at this point in the history
  • Loading branch information
uurien authored and tlhunter committed Jun 23, 2023
1 parent b20a982 commit 0ee7de4
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/iast/analyzers/analyzers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module.exports = {
'INSECURE_COOKIE_ANALYZER': require('./insecure-cookie-analyzer'),
'LDAP_ANALYZER': require('./ldap-injection-analyzer'),
'NO_HTTPONLY_COOKIE_ANALYZER': require('./no-httponly-cookie-analyzer'),
'NO_SAMESITE_COOKIE_ANALYZER': require('./no-samesite-cookie-analyzer'),
'PATH_TRAVERSAL_ANALYZER': require('./path-traversal-analyzer'),
'SQL_INJECTION_ANALYZER': require('./sql-injection-analyzer'),
'SSRF': require('./ssrf-analyzer'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict'

const { NO_SAMESITE_COOKIE } = require('../vulnerabilities')
const CookieAnalyzer = require('./cookie-analyzer')

class NoSamesiteCookieAnalyzer extends CookieAnalyzer {
constructor () {
super(NO_SAMESITE_COOKIE, 'SameSite=strict')
}
}

module.exports = new NoSamesiteCookieAnalyzer()
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/iast/vulnerabilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module.exports = {
INSECURE_COOKIE: 'INSECURE_COOKIE',
LDAP_INJECTION: 'LDAP_INJECTION',
NO_HTTPONLY_COOKIE: 'NO_HTTPONLY_COOKIE',
NO_SAMESITE_COOKIE: 'NO_SAMESITE_COOKIE',
PATH_TRAVERSAL: 'PATH_TRAVERSAL',
SQL_INJECTION: 'SQL_INJECTION',
SSRF: 'SSRF',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
'use strict'

const { prepareTestServerForIastInExpress } = require('../utils')
const fs = require('fs')
const os = require('os')
const path = require('path')
describe('no SameSite cookie vulnerability', () => {
let setCookieFunctions
const setCookieFunctionsFilename = 'set-cookie-express-functions.js'
const setCookieFunctionsPath = path.join(os.tmpdir(), setCookieFunctionsFilename)

before(() => {
fs.copyFileSync(path.join(__dirname, 'resources', 'set-cookie-express-functions.js'), setCookieFunctionsPath)
setCookieFunctions = require(setCookieFunctionsPath)
})

after(() => {
fs.unlinkSync(setCookieFunctionsPath)
})

withVersions('express', 'express', '>=4.15.0', version => {
// Oldest express4 versions do not support sameSite property in cookie method
prepareTestServerForIastInExpress('in express', version,
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
testThatRequestHasVulnerability((req, res) => {
setCookieFunctions.insecureWithResCookieMethod('insecure', 'cookie', res)
}, 'NO_SAMESITE_COOKIE', {
occurrences: 1,
location: {
path: setCookieFunctionsFilename,
line: 4
}
})

testThatRequestHasVulnerability((req, res) => {
setCookieFunctions.insecureWithResCookieMethod('insecure', 'cookie', res)
setCookieFunctions.insecureWithResCookieMethod('insecure2', 'cookie2', res)
}, 'NO_SAMESITE_COOKIE', {
occurrences: 2,
location: {
path: setCookieFunctionsFilename,
line: 4
}
})

testThatRequestHasVulnerability((req, res) => {
setCookieFunctions.insecureWithResHeaderMethod('insecure', 'cookie', res)
}, 'NO_SAMESITE_COOKIE', {
occurrences: 1,
location: {
path: setCookieFunctionsFilename,
line: 8
}
})

testThatRequestHasVulnerability((req, res) => {
setCookieFunctions.insecureWithResHeaderMethodWithArray('insecure', 'cookie', 'insecure2', 'cookie2', res)
}, 'NO_SAMESITE_COOKIE', {
occurrences: 2,
location: {
path: setCookieFunctionsFilename,
line: 12
}
})

testThatRequestHasNoVulnerability((req, res) => {
res.cookie('secure', 'cookie', { secure: true, httpOnly: true, sameSite: 'strict' })
res.cookie('secure', 'cookie', { secure: true, httpOnly: true, sameSite: true })
res.clearCookie('deleteinsecure')
res.header('set-cookie', 'other=secure; Secure; HttpOnly; SameSite=strict')
res.header('set-cookie', ['other=safe; ; SameSite=strict', 'more=safe2; Secure; HttpOnly; SameSite=strict'])
}, 'NO_SAMESITE_COOKIE')
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict'

const { prepareTestServerForIast } = require('../utils')
const Analyzer = require('../../../../src/appsec/iast/analyzers/vulnerability-analyzer')
const { NO_SAMESITE_COOKIE } = require('../../../../src/appsec/iast/vulnerabilities')
const analyzer = new Analyzer()

describe('no SameSite cookie analyzer', () => {
it('Expected vulnerability identifier', () => {
expect(NO_SAMESITE_COOKIE).to.be.equals('NO_SAMESITE_COOKIE')
})

// 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 = {
enabled: true,
requestSampling: 100,
maxConcurrentRequests: 1,
maxContextOperations: 1
}
prepareTestServerForIast('no HttpOnly cookie analyzer',
(testThatRequestHasVulnerability, testThatRequestHasNoVulnerability) => {
testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', 'key=value')
}, NO_SAMESITE_COOKIE, 1, function (vulnerabilities) {
expect(vulnerabilities[0].evidence.value).to.be.equals('key')
expect(vulnerabilities[0].hash).to.be.equals(analyzer._createHash('NO_SAMESITE_COOKIE:key'))
})

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value'])
}, NO_SAMESITE_COOKIE, 1)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value; SameSite=Lax'])
}, NO_SAMESITE_COOKIE, 1)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value; SameSite=None'])
}, NO_SAMESITE_COOKIE, 1)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value', 'key2=value2'])
}, NO_SAMESITE_COOKIE, 2)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value', 'key2=value2; Secure'])
}, NO_SAMESITE_COOKIE, 2)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value', 'key2=value2; SameSite=strict'])
}, NO_SAMESITE_COOKIE, 1)

testThatRequestHasVulnerability((req, res) => {
res.setHeader('set-cookie', ['key=value; SameSite=strict', 'key2=value2; Secure'])
}, NO_SAMESITE_COOKIE, 1)

testThatRequestHasNoVulnerability((req, res) => {
res.setHeader('set-cookie', 'key=value; SameSite=strict')
}, NO_SAMESITE_COOKIE)

testThatRequestHasNoVulnerability((req, res) => {
res.setHeader('set-cookie', 'key=')
}, NO_SAMESITE_COOKIE)
}, iastConfig)
})

0 comments on commit 0ee7de4

Please sign in to comment.