Skip to content

Commit 7674d8c

Browse files
hjr3hjr3
authored
Fix pg prototype pollution via server supplied column names (#3656)
* fix(pg-connection-string): prototype pollution via query strings * fix(pg): prototype pollution via server-supplied column names Fixes #3654
1 parent 1025d12 commit 7674d8c

5 files changed

Lines changed: 156 additions & 11 deletions

File tree

‎packages/pg-connection-string/index.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function parse(str, options = {}) {
1414

1515
// Check for empty host in URL
1616

17-
const config = {}
17+
const config = Object.create(null)
1818
let result
1919
let dummyHost = false
2020
if (/ |%[^a-f0-9]|%[a-f0-9][^a-f0-9]/i.test(str)) {
@@ -164,7 +164,7 @@ function toConnectionOptions(sslConfig) {
164164
}
165165

166166
return c
167-
}, {})
167+
}, Object.create(null))
168168

169169
return connectionOptions
170170
}
@@ -200,7 +200,7 @@ function toClientConfig(config) {
200200
}
201201

202202
return c
203-
}, {})
203+
}, Object.create(null))
204204

205205
return poolConfig
206206
}

‎packages/pg-connection-string/test/clientConfig.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('toClientConfig', function () {
4646
const config = parse('pg:///?sslmode=no-verify')
4747
const clientConfig = toClientConfig(config)
4848

49-
clientConfig.ssl?.should.deep.equal({
49+
expect(clientConfig.ssl).to.deep.equal({
5050
rejectUnauthorized: false,
5151
})
5252
})
@@ -55,14 +55,14 @@ describe('toClientConfig', function () {
5555
const config = parse('pg:///?sslmode=verify-ca')
5656
const clientConfig = toClientConfig(config)
5757

58-
clientConfig.ssl?.should.deep.equal({})
58+
expect(clientConfig.ssl).to.deep.equal({})
5959
})
6060

6161
it('converts other sslmode options', function () {
6262
const config = parse('pg:///?sslmode=verify-ca')
6363
const clientConfig = toClientConfig(config)
6464

65-
clientConfig.ssl?.should.deep.equal({})
65+
expect(clientConfig.ssl).to.deep.equal({})
6666
})
6767

6868
it('converts ssl cert options', function () {
@@ -77,7 +77,7 @@ describe('toClientConfig', function () {
7777
const config = parse(connectionString)
7878
const clientConfig = toClientConfig(config)
7979

80-
clientConfig.ssl?.should.deep.equal({
80+
expect(clientConfig.ssl).to.deep.equal({
8181
ca: 'example ca\n',
8282
cert: 'example cert\n',
8383
key: 'example key\n',
@@ -106,9 +106,9 @@ describe('toClientConfig', function () {
106106

107107
const clientConfig = toClientConfig(config)
108108

109-
clientConfig.host?.should.equal('boom')
110-
clientConfig.database?.should.equal('lala')
111-
clientConfig.ssl?.should.deep.equal({})
109+
expect(clientConfig.host).to.equal('boom')
110+
expect(clientConfig.database).to.equal('lala')
111+
expect(clientConfig.ssl).to.deep.equal({})
112112
})
113113
})
114114

‎packages/pg-connection-string/test/parse.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,4 +467,38 @@ describe('parse', function () {
467467
const subject = parse(connectionString)
468468
subject.port?.should.equal('1234')
469469
})
470+
471+
describe('prototype pollution protection', function () {
472+
it('returns object with null prototype', function () {
473+
const subject = parse('postgres://localhost/db')
474+
expect(Object.getPrototypeOf(subject)).to.equal(null)
475+
})
476+
477+
it('__proto__ query parameter is stored as regular property', function () {
478+
const subject = parse('postgres://localhost/db?__proto__=malicious')
479+
expect(Object.getPrototypeOf(subject)).to.equal(null)
480+
expect(subject['__proto__']).to.equal('malicious')
481+
// global Object.prototype should not be affected
482+
expect(({} as any).malicious).to.equal(undefined)
483+
})
484+
485+
it('constructor query parameter is stored as regular property', function () {
486+
const subject = parse('postgres://localhost/db?constructor=evil')
487+
expect(subject.constructor).to.equal('evil')
488+
})
489+
490+
it('prototype query parameter is stored as regular property', function () {
491+
const subject = parse('postgres://localhost/db?prototype=evil')
492+
expect(subject['prototype']).to.equal('evil')
493+
})
494+
495+
it('multiple dangerous query parameters are handled safely', function () {
496+
const subject = parse('postgres://localhost/db?__proto__=a&constructor=b&prototype=c&toString=d')
497+
expect(Object.getPrototypeOf(subject)).to.equal(null)
498+
expect(subject['__proto__']).to.equal('a')
499+
expect(subject.constructor).to.equal('b')
500+
expect(subject['prototype']).to.equal('c')
501+
expect(subject['toString']).to.equal('d')
502+
})
503+
})
470504
})

‎packages/pg/lib/result.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Result {
8989
this._parsers = new Array(fieldDescriptions.length)
9090
}
9191

92-
const row = {}
92+
const row = Object.create(null)
9393

9494
for (let i = 0; i < fieldDescriptions.length; i++) {
9595
const desc = fieldDescriptions[i]

‎packages/pg/test/unit/result-tests.js‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
'use strict'
2+
const helper = require('./test-helper')
3+
const assert = require('assert')
4+
const suite = new helper.Suite()
5+
const test = suite.test.bind(suite)
6+
7+
const Result = require('../../lib/result')
8+
9+
test('__proto__ column name does not pollute prototype', function () {
10+
const result = new Result()
11+
result.addFields([
12+
{ name: '__proto__', dataTypeID: 25, format: 'text' },
13+
{ name: 'id', dataTypeID: 23, format: 'text' },
14+
])
15+
const row = result.parseRow(['malicious', '1'])
16+
17+
// __proto__ should be a regular property, not affect prototype chain
18+
assert.strictEqual(row['__proto__'], 'malicious')
19+
assert.strictEqual(row.id, 1)
20+
21+
// global Object.prototype should not be affected
22+
assert.strictEqual({}.malicious, undefined)
23+
assert.strictEqual(Object.prototype.malicious, undefined)
24+
})
25+
26+
test('__proto__ column with object value does not inject prototype', function () {
27+
// custom type parser that returns objects (like JSON)
28+
const customTypes = {
29+
getTypeParser: () => (val) => JSON.parse(val),
30+
}
31+
const result = new Result('object', customTypes)
32+
result.addFields([
33+
{ name: '__proto__', dataTypeID: 114, format: 'text' },
34+
{ name: 'id', dataTypeID: 23, format: 'text' },
35+
])
36+
37+
const maliciousPayload = JSON.stringify({ isAdmin: true, role: 'admin' })
38+
const row = result.parseRow([maliciousPayload, '1'])
39+
40+
// __proto__ should be stored as a regular property
41+
assert.deepStrictEqual(row['__proto__'], { isAdmin: true, role: 'admin' })
42+
43+
// the row should NOT inherit from the malicious payload
44+
assert.strictEqual('isAdmin' in row, false)
45+
assert.strictEqual('role' in row, false)
46+
})
47+
48+
test('constructor column name is safely stored as property', function () {
49+
const result = new Result()
50+
result.addFields([
51+
{ name: 'constructor', dataTypeID: 25, format: 'text' },
52+
{ name: 'id', dataTypeID: 23, format: 'text' },
53+
])
54+
const row = result.parseRow(['malicious', '1'])
55+
56+
assert.strictEqual(row.constructor, 'malicious')
57+
assert.strictEqual(row.id, 1)
58+
})
59+
60+
test('hasOwnProperty column name is safely stored as property', function () {
61+
const result = new Result()
62+
result.addFields([
63+
{ name: 'hasOwnProperty', dataTypeID: 25, format: 'text' },
64+
{ name: 'data', dataTypeID: 25, format: 'text' },
65+
])
66+
const row = result.parseRow(['not_a_function', 'value'])
67+
68+
assert.strictEqual(row.hasOwnProperty, 'not_a_function')
69+
assert.strictEqual(row.data, 'value')
70+
71+
// can still check properties using Object.prototype.hasOwnProperty.call
72+
assert.strictEqual(Object.prototype.hasOwnProperty.call(row, 'data'), true)
73+
})
74+
75+
test('toString column name is safely stored as property', function () {
76+
const result = new Result()
77+
result.addFields([{ name: 'toString', dataTypeID: 25, format: 'text' }])
78+
const row = result.parseRow(['not_a_function'])
79+
80+
assert.strictEqual(row.toString, 'not_a_function')
81+
})
82+
83+
test('prototype column name is safely stored as property', function () {
84+
const result = new Result()
85+
result.addFields([
86+
{ name: 'prototype', dataTypeID: 25, format: 'text' },
87+
{ name: 'id', dataTypeID: 23, format: 'text' },
88+
])
89+
const row = result.parseRow(['value', '1'])
90+
91+
assert.strictEqual(row.prototype, 'value')
92+
assert.strictEqual(row.id, 1)
93+
})
94+
95+
test('multiple dangerous column names handled safely', function () {
96+
const result = new Result()
97+
result.addFields([
98+
{ name: '__proto__', dataTypeID: 25, format: 'text' },
99+
{ name: 'constructor', dataTypeID: 25, format: 'text' },
100+
{ name: 'prototype', dataTypeID: 25, format: 'text' },
101+
{ name: '__defineGetter__', dataTypeID: 25, format: 'text' },
102+
{ name: 'id', dataTypeID: 23, format: 'text' },
103+
])
104+
const row = result.parseRow(['a', 'b', 'c', 'd', '1'])
105+
106+
assert.strictEqual(row['__proto__'], 'a')
107+
assert.strictEqual(row.constructor, 'b')
108+
assert.strictEqual(row.prototype, 'c')
109+
assert.strictEqual(row['__defineGetter__'], 'd')
110+
assert.strictEqual(row.id, 1)
111+
})

0 commit comments

Comments
 (0)