Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exploit prevention SQLi in pg #4566

Merged
merged 28 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
daefc81
pg query with abortcontroller
uurien Aug 1, 2024
d46be5e
Adding instrumentation tests
uurien Aug 1, 2024
4dd4685
Changing instrumentation for query object and adding tests
uurien Aug 2, 2024
15ede9b
Add tests for pg.pool
uurien Aug 2, 2024
a8906cf
Implement abortController for pg.pool
uurien Aug 2, 2024
6145de2
Call to waf with pg query
uurien Aug 5, 2024
56014d6
Add pg.Pool blocking tests
uurien Aug 5, 2024
3570cec
Analyze also in pg.Pool and ignore pg.Client when it is already analyzed
uurien Aug 6, 2024
dc1cbd6
Refactor - split rasp.js and move it to rasp directory
uurien Aug 6, 2024
c7b82f9
Add integration tests for pg and unhandled promise
uurien Aug 6, 2024
3c48698
Cleaning code
uurien Aug 6, 2024
ff05f99
Add timeout to integration tests
uurien Aug 7, 2024
48b39ff
Small fix in test
uurien Aug 8, 2024
0d8920c
Add new integration test
uurien Aug 8, 2024
a9c48b3
Merge branch 'master' into ugaitz/rasp-sql-pg
uurien Sep 2, 2024
d77f60a
Apply some PR suggestions
uurien Sep 3, 2024
a3ea4fc
Change approach to prevent double calls
uurien Sep 5, 2024
4da334c
Merge branch 'master' into ugaitz/rasp-sql-pg
uurien Sep 5, 2024
35763ec
Do not prevent double calls if an input address is updated between calls
uurien Sep 5, 2024
03a9435
Small fixes
uurien Sep 6, 2024
a8902c5
Merge branch 'master' into ugaitz/rasp-sql-pg
uurien Sep 6, 2024
636b3ed
Do not use Array.concat
uurien Sep 9, 2024
3b111f5
Use assert instead of checking error message manually
uurien Sep 9, 2024
70f195d
require missing assert
uurien Sep 9, 2024
efa203a
Apply comments in the PR
uurien Sep 9, 2024
38c56cf
Update packages/dd-trace/test/appsec/waf/waf_context_wrapper.spec.js
uurien Sep 11, 2024
698e1b8
Merge branch 'master' into ugaitz/rasp-sql-pg
uurien Sep 11, 2024
d93eabe
Update packages/dd-trace/test/appsec/rasp/sql_injection.spec.js
uurien Sep 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions packages/datadog-instrumentations/src/pg.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,59 @@ function wrapQuery (query) {
}

return asyncResource.runInAsyncScope(() => {
const abortController = new AbortController()

startCh.publish({
params: this.connectionParameters,
query: pgQuery,
processId
processId,
abortController
})

arguments[0] = pgQuery

const finish = asyncResource.bind(function (error) {
if (error) {
errorCh.publish(error)
}
finishCh.publish()
})

if (abortController.signal.aborted) {
const error = abortController.signal.reason || new Error('Aborted')

// eslint-disable-next-line max-len
// Based on: https://github.com/brianc/node-postgres/blob/54eb0fa216aaccd727765641e7d1cf5da2bc483d/packages/pg/lib/client.js#L510
const reusingQuery = typeof pgQuery.submit === 'function'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth putting this comment directly in the code

const callback = arguments[arguments.length - 1]

finish(error)

if (reusingQuery) {
if (!pgQuery.callback && typeof callback === 'function') {
pgQuery.callback = callback
}

if (pgQuery.callback) {
pgQuery.callback(error)
} else {
process.nextTick(() => {
pgQuery.emit('error', error)
simon-id marked this conversation as resolved.
Show resolved Hide resolved
})
}

return pgQuery
}

if (typeof callback === 'function') {
simon-id marked this conversation as resolved.
Show resolved Hide resolved
callback(error)

return
}

return Promise.reject(error)
}

arguments[0] = pgQuery

const retval = query.apply(this, arguments)
const queryQueue = this.queryQueue || this._queryQueue
const activeQuery = this.activeQuery || this._activeQuery
Expand Down Expand Up @@ -112,15 +150,32 @@ function wrapPoolQuery (query) {
const pgQuery = arguments[0] !== null && typeof arguments[0] === 'object' ? arguments[0] : { text: arguments[0] }

return asyncResource.runInAsyncScope(() => {
const abortController = new AbortController()

startPoolQueryCh.publish({
query: pgQuery
query: pgQuery,
abortController
})

const finish = asyncResource.bind(function () {
finishPoolQueryCh.publish()
})

const cb = arguments[arguments.length - 1]

if (abortController.signal.aborted) {
const error = abortController.signal.reason || new Error('Aborted')
finish()

if (typeof cb === 'function') {
cb(error)

return
} else {
return Promise.reject(error)
}
}

if (typeof cb === 'function') {
arguments[arguments.length - 1] = shimmer.wrapFunction(cb, cb => function () {
finish()
Expand Down
244 changes: 244 additions & 0 deletions packages/datadog-instrumentations/test/pg.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
'use strict'

const agent = require('../../dd-trace/test/plugins/agent')
const dc = require('dc-polyfill')
const { assert } = require('chai')

const clients = {
pg: pg => pg.Client
}

if (process.env.PG_TEST_NATIVE === 'true') {
clients['pg.native'] = pg => pg.native.Client
}

describe('pg instrumentation', () => {
withVersions('pg', 'pg', version => {
const queryClientStartChannel = dc.channel('apm:pg:query:start')
const queryPoolStartChannel = dc.channel('datadog:pg:pool:query:start')

let pg
let Query

function abortQuery ({ abortController }) {
const error = new Error('Test')
abortController.abort(error)

if (!abortController.signal.reason) {
abortController.signal.reason = error
}
}

before(() => {
return agent.load(['pg'])
})

describe('pg.Client', () => {
Object.keys(clients).forEach(implementation => {
describe(implementation, () => {
let client

beforeEach(done => {
pg = require(`../../../versions/pg@${version}`).get()
const Client = clients[implementation](pg)
Query = Client.Query

client = new Client({
host: '127.0.0.1',
user: 'postgres',
password: 'postgres',
database: 'postgres',
application_name: 'test'
})

client.connect(err => done(err))
})

afterEach(() => {
client.end()
})

describe('abortController', () => {
afterEach(() => {
if (queryClientStartChannel.hasSubscribers) {
queryClientStartChannel.unsubscribe(abortQuery)
}
})

describe('using callback', () => {
it('Should not fail if it is not aborted', (done) => {
client.query('SELECT 1', (err) => {
done(err)
})
})

it('Should abort query', (done) => {
queryClientStartChannel.subscribe(abortQuery)

client.query('SELECT 1', (err) => {
assert.propertyVal(err, 'message', 'Test')
done()
})
})
})

describe('using promise', () => {
it('Should not fail if it is not aborted', async () => {
await client.query('SELECT 1')
})

it('Should abort query', async () => {
queryClientStartChannel.subscribe(abortQuery)

try {
await client.query('SELECT 1')
} catch (err) {
assert.propertyVal(err, 'message', 'Test')

return
}

throw new Error('Query was not aborted')
})
})

describe('using query object', () => {
describe('without callback', () => {
it('Should not fail if it is not aborted', (done) => {
const query = new Query('SELECT 1')

client.query(query)

query.on('end', () => {
done()
})
})

it('Should abort query', (done) => {
queryClientStartChannel.subscribe(abortQuery)

const query = new Query('SELECT 1')

client.query(query)

query.on('error', err => {
assert.propertyVal(err, 'message', 'Test')
done()
})

query.on('end', () => {
done(new Error('Query was not aborted'))
})
})
})

describe('with callback in query object', () => {
it('Should not fail if it is not aborted', (done) => {
const query = new Query('SELECT 1')
query.callback = (err) => {
done(err)
}

client.query(query)
})

it('Should abort query', (done) => {
queryClientStartChannel.subscribe(abortQuery)

const query = new Query('SELECT 1')
query.callback = err => {
assert.propertyVal(err, 'message', 'Test')
done()
}

client.query(query)
})
})

describe('with callback in query parameter', () => {
it('Should not fail if it is not aborted', (done) => {
const query = new Query('SELECT 1')

client.query(query, (err) => {
done(err)
})
})

it('Should abort query', (done) => {
queryClientStartChannel.subscribe(abortQuery)

const query = new Query('SELECT 1')

client.query(query, err => {
assert.propertyVal(err, 'message', 'Test')
done()
})
})
})
})
})
})
})
})

describe('pg.Pool', () => {
let pool

beforeEach(() => {
const { Pool } = require(`../../../versions/pg@${version}`).get()

pool = new Pool({
host: '127.0.0.1',
user: 'postgres',
password: 'postgres',
database: 'postgres',
application_name: 'test'
})
})

describe('abortController', () => {
afterEach(() => {
if (queryPoolStartChannel.hasSubscribers) {
queryPoolStartChannel.unsubscribe(abortQuery)
}
})

describe('using callback', () => {
it('Should not fail if it is not aborted', (done) => {
pool.query('SELECT 1', (err) => {
done(err)
})
})

it('Should abort query', (done) => {
queryPoolStartChannel.subscribe(abortQuery)

pool.query('SELECT 1', (err) => {
assert.propertyVal(err, 'message', 'Test')
done()
})
})
})

describe('using promise', () => {
it('Should not fail if it is not aborted', async () => {
await pool.query('SELECT 1')
})

it('Should abort query', async () => {
queryPoolStartChannel.subscribe(abortQuery)

try {
await pool.query('SELECT 1')
} catch (err) {
assert.propertyVal(err, 'message', 'Test')
return
}

throw new Error('Query was not aborted')
})
})
})
})
})
})
4 changes: 3 additions & 1 deletion packages/dd-trace/src/appsec/addresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@ module.exports = {
USER_ID: 'usr.id',
WAF_CONTEXT_PROCESSOR: 'waf.context.processor',

HTTP_OUTGOING_URL: 'server.io.net.url'
HTTP_OUTGOING_URL: 'server.io.net.url',
DB_STATEMENT: 'server.db.statement',
DB_SYSTEM: 'server.db.system'
}
6 changes: 4 additions & 2 deletions packages/dd-trace/src/appsec/channels.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module.exports = {
responseWriteHead: dc.channel('apm:http:server:response:writeHead:start'),
httpClientRequestStart: dc.channel('apm:http:client:request:start'),
responseSetHeader: dc.channel('datadog:http:server:response:set-header:start'),
setUncaughtExceptionCaptureCallbackStart: dc.channel('datadog:process:setUncaughtExceptionCaptureCallback:start')

setUncaughtExceptionCaptureCallbackStart: dc.channel('datadog:process:setUncaughtExceptionCaptureCallback:start'),
pgQueryStart: dc.channel('apm:pg:query:start'),
pgPoolQueryStart: dc.channel('datadog:pg:pool:query:start'),
wafRunFinished: dc.channel('datadog:waf:run:finish')
}
Loading
Loading