Skip to content

Commit 81a97db

Browse files
committed
fix(pg): prototype pollution via server-supplied column names
Fixes #3654
1 parent b7f469d commit 81a97db

File tree

2 files changed

+112
-1
lines changed

2 files changed

+112
-1
lines changed

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]
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)