Skip to content

Commit b64e7e4

Browse files
authored
fix: avoid prototype collisions in parseHeaders (#4923)
1 parent deba679 commit b64e7e4

3 files changed

Lines changed: 116 additions & 12 deletions

File tree

benchmarks/core/parse-headers.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ bench('noop', () => {})
8787
bench('noop', () => {})
8888
bench('noop', () => {})
8989

90-
group('parseHeaders', () => {
90+
group(() => {
9191
bench('parseHeaders', () => {
9292
for (let i = 0; i < headers.length; ++i) {
9393
parseHeaders(headers[i])

lib/core/util.js

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -440,19 +440,39 @@ function parseHeaders (headers, obj) {
440440
const key = headerNameToString(headers[i])
441441
let val = obj[key]
442442

443-
if (val) {
444-
if (typeof val === 'string') {
445-
val = [val]
446-
obj[key] = val
447-
}
448-
val.push(headers[i + 1].toString('latin1'))
449-
} else {
450-
const headersValue = headers[i + 1]
451-
if (typeof headersValue === 'string') {
452-
obj[key] = headersValue
443+
if (val !== undefined) {
444+
if (!Object.hasOwn(obj, key)) {
445+
const headersValue = typeof headers[i + 1] === 'string'
446+
? headers[i + 1]
447+
: Array.isArray(headers[i + 1])
448+
? headers[i + 1].map(x => x.toString('latin1'))
449+
: headers[i + 1].toString('latin1')
450+
451+
if (key === '__proto__') {
452+
Object.defineProperty(obj, key, {
453+
value: headersValue,
454+
enumerable: true,
455+
configurable: true,
456+
writable: true
457+
})
458+
} else {
459+
obj[key] = headersValue
460+
}
453461
} else {
454-
obj[key] = Array.isArray(headersValue) ? headersValue.map(x => x.toString('latin1')) : headersValue.toString('latin1')
462+
if (typeof val === 'string') {
463+
val = [val]
464+
obj[key] = val
465+
}
466+
val.push(headers[i + 1].toString('latin1'))
455467
}
468+
} else {
469+
const headersValue = typeof headers[i + 1] === 'string'
470+
? headers[i + 1]
471+
: Array.isArray(headers[i + 1])
472+
? headers[i + 1].map(x => x.toString('latin1'))
473+
: headers[i + 1].toString('latin1')
474+
475+
obj[key] = headersValue
456476
}
457477
}
458478

test/prototype-headers.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
'use strict'
2+
3+
const { test } = require('node:test')
4+
const assert = require('node:assert')
5+
const { promisify } = require('node:util')
6+
const net = require('node:net')
7+
const { Client } = require('..')
8+
9+
function createRawServer (response) {
10+
return net.createServer((socket) => {
11+
socket.once('data', () => {
12+
socket.end(response)
13+
})
14+
})
15+
}
16+
17+
test('request handles response headers that shadow Object.prototype', async (t) => {
18+
const server = createRawServer([
19+
'HTTP/1.1 200 OK',
20+
'__proto__: pwned',
21+
'constructor: built-in',
22+
'content-length: 2',
23+
'connection: close',
24+
'',
25+
'OK'
26+
].join('\r\n'))
27+
28+
t.after(() => {
29+
server.closeAllConnections?.()
30+
server.close()
31+
})
32+
33+
await promisify(server.listen.bind(server))(0)
34+
35+
const client = new Client(`http://localhost:${server.address().port}`)
36+
t.after(() => client.close())
37+
38+
const { statusCode, headers, body } = await client.request({
39+
path: '/',
40+
method: 'GET'
41+
})
42+
43+
assert.strictEqual(statusCode, 200)
44+
assert.strictEqual(Object.getOwnPropertyDescriptor(headers, '__proto__').value, 'pwned')
45+
assert.strictEqual(Object.getOwnPropertyDescriptor(headers, 'constructor').value, 'built-in')
46+
assert.strictEqual(await body.text(), 'OK')
47+
})
48+
49+
test('request handles response trailers that shadow Object.prototype', async (t) => {
50+
const server = createRawServer([
51+
'HTTP/1.1 200 OK',
52+
'transfer-encoding: chunked',
53+
'trailer: __proto__, constructor',
54+
'connection: close',
55+
'',
56+
'2',
57+
'OK',
58+
'0',
59+
'__proto__: trailer',
60+
'constructor: built-in-trailer',
61+
'',
62+
''
63+
].join('\r\n'))
64+
65+
t.after(() => {
66+
server.closeAllConnections?.()
67+
server.close()
68+
})
69+
70+
await promisify(server.listen.bind(server))(0)
71+
72+
const client = new Client(`http://localhost:${server.address().port}`)
73+
t.after(() => client.close())
74+
75+
const { statusCode, trailers, body } = await client.request({
76+
path: '/',
77+
method: 'GET'
78+
})
79+
80+
assert.strictEqual(statusCode, 200)
81+
assert.strictEqual(await body.text(), 'OK')
82+
assert.strictEqual(Object.getOwnPropertyDescriptor(trailers, '__proto__').value, 'trailer')
83+
assert.strictEqual(Object.getOwnPropertyDescriptor(trailers, 'constructor').value, 'built-in-trailer')
84+
})

0 commit comments

Comments
 (0)