Skip to content

Commit 21436f0

Browse files
authored
http: make req.headers have a null prototype
Makes IncomingMessage.prototype.headers and trailers have a null prototype, matching the existing behavior of headersDistinct and trailersDistinct. Fixes prototype pollution concerns where headers like __proto__ could be interpreted as prototype manipulation. Refs: #61771 PR-URL: #62900 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Gürgün Dayıoğlu <[email protected]> Reviewed-By: Tim Perry <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: René <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
1 parent 10ae641 commit 21436f0

15 files changed

Lines changed: 186 additions & 65 deletions

doc/api/http.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,6 +2864,9 @@ The request/response headers object.
28642864

28652865
Key-value pairs of header names and values. Header names are lower-cased.
28662866

2867+
The object has a null prototype and should not be accessed using the `in`
2868+
operator.
2869+
28672870
```js
28682871
// Prints something like:
28692872
//
@@ -2901,6 +2904,9 @@ added:
29012904
Similar to [`message.headers`][], but there is no join logic and the values are
29022905
always arrays of strings, even for headers received just once.
29032906

2907+
The object has a null prototype and should not be accessed using the `in`
2908+
operator.
2909+
29042910
```js
29052911
// Prints something like:
29062912
//
@@ -3087,6 +3093,9 @@ added: v0.3.0
30873093

30883094
The request/response trailers object. Only populated at the `'end'` event.
30893095

3096+
The object has a null prototype and should not be accessed using the `in`
3097+
operator.
3098+
30903099
### `message.trailersDistinct`
30913100

30923101
<!-- YAML
@@ -3101,6 +3110,9 @@ Similar to [`message.trailers`][], but there is no join logic and the values are
31013110
always arrays of strings, even for headers received just once.
31023111
Only populated at the `'end'` event.
31033112

3113+
The object has a null prototype and should not be accessed using the `in`
3114+
operator.
3115+
31043116
### `message.url`
31053117

31063118
<!-- YAML

doc/api/http2.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4120,6 +4120,9 @@ The request/response headers object.
41204120

41214121
Key-value pairs of header names and values. Header names are lower-cased.
41224122

4123+
The object has a null prototype and should not be accessed using the `in`
4124+
operator.
4125+
41234126
```js
41244127
// Prints something like:
41254128
//
@@ -4279,6 +4282,9 @@ added: v8.4.0
42794282

42804283
The request/response trailers object. Only populated at the `'end'` event.
42814284

4285+
The object has a null prototype and should not be accessed using the `in`
4286+
operator.
4287+
42824288
#### `request.url`
42834289

42844290
<!-- YAML

lib/_http_incoming.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headers', {
112112
__proto__: null,
113113
get: function() {
114114
if (!this[kHeaders]) {
115-
this[kHeaders] = {};
115+
this[kHeaders] = { __proto__: null };
116116

117117
const src = this.rawHeaders;
118118
const dst = this[kHeaders];
@@ -152,7 +152,7 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailers', {
152152
__proto__: null,
153153
get: function() {
154154
if (!this[kTrailers]) {
155-
this[kTrailers] = {};
155+
this[kTrailers] = { __proto__: null };
156156

157157
const src = this.rawTrailers;
158158
const dst = this[kTrailers];

lib/internal/http2/compat.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ class Http2ServerRequest extends Readable {
319319
// initialization using Object.create(null) in HTTP/2 is intentional.
320320
this[kHeaders] = headers;
321321
this[kRawHeaders] = rawHeaders;
322-
this[kTrailers] = {};
322+
this[kTrailers] = { __proto__: null };
323323
this[kRawTrailers] = [];
324324
this[kStream] = stream;
325325
this[kAborted] = false;

test/parallel/test-http-blank-header.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,10 @@ const net = require('net');
2828
const server = http.createServer(common.mustCall((req, res) => {
2929
assert.strictEqual(req.method, 'GET');
3030
assert.strictEqual(req.url, '/blah');
31-
assert.deepStrictEqual(req.headers, {
32-
host: 'example.org:443',
33-
origin: 'http://example.org',
34-
cookie: ''
35-
});
31+
assert.deepStrictEqual(req.headers, { __proto__: null,
32+
host: 'example.org:443',
33+
origin: 'http://example.org',
34+
cookie: '' });
3635
}));
3736

3837

test/parallel/test-http-client-headers-array.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ const http = require('http');
77

88
function execute(options) {
99
http.createServer(common.mustCall(function(req, res) {
10-
const expectHeaders = {
11-
'x-foo': 'boom',
12-
'cookie': 'a=1; b=2; c=3',
13-
'connection': 'keep-alive',
14-
'host': 'example.com',
15-
};
10+
const expectHeaders = { '__proto__': null,
11+
'x-foo': 'boom',
12+
'cookie': 'a=1; b=2; c=3',
13+
'connection': 'keep-alive',
14+
'host': 'example.com' };
1615

1716
// no Host header when you set headers an array
1817
if (!Array.isArray(options.headers)) {

test/parallel/test-http-content-length.js

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,17 @@ const assert = require('assert');
44
const http = require('http');
55
const Countdown = require('../common/countdown');
66

7-
const expectedHeadersMultipleWrites = {
8-
'connection': 'keep-alive',
9-
'transfer-encoding': 'chunked',
10-
};
7+
const expectedHeadersMultipleWrites = { '__proto__': null,
8+
'connection': 'keep-alive',
9+
'transfer-encoding': 'chunked' };
1110

12-
const expectedHeadersEndWithData = {
13-
'connection': 'keep-alive',
14-
'content-length': String('hello world'.length),
15-
};
11+
const expectedHeadersEndWithData = { '__proto__': null,
12+
'connection': 'keep-alive',
13+
'content-length': String('hello world'.length) };
1614

17-
const expectedHeadersEndNoData = {
18-
'connection': 'keep-alive',
19-
'content-length': '0',
20-
};
15+
const expectedHeadersEndNoData = { '__proto__': null,
16+
'connection': 'keep-alive',
17+
'content-length': '0' };
2118

2219

2320
const countdown = new Countdown(3, () => server.close());
@@ -62,7 +59,9 @@ server.listen(0, common.mustCall(function() {
6259
req.write('hello ');
6360
req.end('world');
6461
req.on('response', common.mustCall((res) => {
65-
assert.deepStrictEqual(res.headers, { ...expectedHeadersMultipleWrites, 'keep-alive': 'timeout=1' });
62+
assert.deepStrictEqual(res.headers, {
63+
'__proto__': null, ...expectedHeadersMultipleWrites, 'keep-alive': 'timeout=1',
64+
});
6665
res.resume();
6766
}));
6867

@@ -74,7 +73,9 @@ server.listen(0, common.mustCall(function() {
7473
req.removeHeader('Date');
7574
req.end('hello world');
7675
req.on('response', common.mustCall((res) => {
77-
assert.deepStrictEqual(res.headers, { ...expectedHeadersEndWithData, 'keep-alive': 'timeout=1' });
76+
assert.deepStrictEqual(res.headers, {
77+
'__proto__': null, ...expectedHeadersEndWithData, 'keep-alive': 'timeout=1',
78+
});
7879
res.resume();
7980
}));
8081

@@ -86,7 +87,7 @@ server.listen(0, common.mustCall(function() {
8687
req.removeHeader('Date');
8788
req.end();
8889
req.on('response', common.mustCall((res) => {
89-
assert.deepStrictEqual(res.headers, { ...expectedHeadersEndNoData, 'keep-alive': 'timeout=1' });
90+
assert.deepStrictEqual(res.headers, { '__proto__': null, ...expectedHeadersEndNoData, 'keep-alive': 'timeout=1' });
9091
res.resume();
9192
}));
9293

test/parallel/test-http-multiple-headers.js

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ const server = createServer(
1919
'Host', host,
2020
'Transfer-Encoding', 'chunked',
2121
]);
22-
assert.deepStrictEqual(req.headers, {
23-
'connection': 'close',
24-
'x-req-a': 'eee, fff, ggg, hhh',
25-
'x-req-b': 'iii; jjj; kkk; lll',
26-
host,
27-
'transfer-encoding': 'chunked'
28-
});
22+
assert.deepStrictEqual(req.headers, { '__proto__': null,
23+
'connection': 'close',
24+
'x-req-a': 'eee, fff, ggg, hhh',
25+
'x-req-b': 'iii; jjj; kkk; lll',
26+
host,
27+
'transfer-encoding': 'chunked' });
2928
assert.deepStrictEqual(req.headersDistinct, Object.assign({ __proto__: null }, {
3029
'connection': ['close'],
3130
'x-req-a': ['eee', 'fff', 'ggg', 'hhh'],
@@ -41,7 +40,7 @@ const server = createServer(
4140
'X-req-Y', 'zzz; www',
4241
]);
4342
assert.deepStrictEqual(
44-
req.trailers, { 'x-req-x': 'xxx, yyy', 'x-req-y': 'zzz; www' }
43+
req.trailers, { '__proto__': null, 'x-req-x': 'xxx, yyy', 'x-req-y': 'zzz; www' }
4544
);
4645
assert.deepStrictEqual(
4746
req.trailersDistinct,
@@ -124,14 +123,13 @@ server.listen(0, common.mustCall(() => {
124123
'x-res-d', 'JJJ; KKK; LLL',
125124
'Transfer-Encoding', 'chunked',
126125
]);
127-
assert.deepStrictEqual(res.headers, {
128-
'x-res-a': 'AAA, BBB, CCC',
129-
'x-res-b': 'DDD; EEE; FFF; GGG',
130-
'connection': 'close',
131-
'x-res-c': 'HHH, III',
132-
'x-res-d': 'JJJ; KKK; LLL',
133-
'transfer-encoding': 'chunked'
134-
});
126+
assert.deepStrictEqual(res.headers, { '__proto__': null,
127+
'x-res-a': 'AAA, BBB, CCC',
128+
'x-res-b': 'DDD; EEE; FFF; GGG',
129+
'connection': 'close',
130+
'x-res-c': 'HHH, III',
131+
'x-res-d': 'JJJ; KKK; LLL',
132+
'transfer-encoding': 'chunked' });
135133
assert.deepStrictEqual(res.headersDistinct, Object.assign({ __proto__: null }, {
136134
'x-res-a': [ 'AAA', 'BBB', 'CCC' ],
137135
'x-res-b': [ 'DDD; EEE; FFF; GGG' ],
@@ -149,7 +147,7 @@ server.listen(0, common.mustCall(() => {
149147
]);
150148
assert.deepStrictEqual(
151149
res.trailers,
152-
{ 'x-res-x': 'XXX, YYY', 'x-res-y': 'ZZZ; WWW' }
150+
{ '__proto__': null, 'x-res-x': 'XXX, YYY', 'x-res-y': 'ZZZ; WWW' }
153151
);
154152
assert.deepStrictEqual(
155153
res.trailersDistinct,

test/parallel/test-http-raw-headers.js

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,11 @@ http.createServer(common.mustCall(function(req, res) {
3636
'Connection',
3737
'keep-alive',
3838
];
39-
const expectHeaders = {
40-
'host': `localhost:${this.address().port}`,
41-
'transfer-encoding': 'CHUNKED',
42-
'x-bar': 'yoyoyo',
43-
'connection': 'keep-alive'
44-
};
39+
const expectHeaders = { '__proto__': null,
40+
'host': `localhost:${this.address().port}`,
41+
'transfer-encoding': 'CHUNKED',
42+
'x-bar': 'yoyoyo',
43+
'connection': 'keep-alive' };
4544
const expectRawTrailers = [
4645
'x-bAr',
4746
'yOyOyOy',
@@ -52,7 +51,7 @@ http.createServer(common.mustCall(function(req, res) {
5251
'X-baR',
5352
'OyOyOyO',
5453
];
55-
const expectTrailers = { 'x-bar': 'yOyOyOy, OyOyOyO, yOyOyOy, OyOyOyO' };
54+
const expectTrailers = { '__proto__': null, 'x-bar': 'yOyOyOy, OyOyOyO, yOyOyOy, OyOyOyO' };
5655

5756
this.close();
5857

@@ -98,13 +97,12 @@ http.createServer(common.mustCall(function(req, res) {
9897
'Transfer-Encoding',
9998
'chunked',
10099
];
101-
const expectHeaders = {
102-
'keep-alive': 'timeout=1',
103-
'trailer': 'x-foo',
104-
'date': null,
105-
'connection': 'keep-alive',
106-
'transfer-encoding': 'chunked'
107-
};
100+
const expectHeaders = { '__proto__': null,
101+
'keep-alive': 'timeout=1',
102+
'trailer': 'x-foo',
103+
'date': null,
104+
'connection': 'keep-alive',
105+
'transfer-encoding': 'chunked' };
108106
res.rawHeaders[5] = null;
109107
res.headers.date = null;
110108
assert.deepStrictEqual(res.rawHeaders, expectRawHeaders);
@@ -120,7 +118,7 @@ http.createServer(common.mustCall(function(req, res) {
120118
'X-foO',
121119
'OxOxOxO',
122120
];
123-
const expectTrailers = { 'x-foo': 'xOxOxOx, OxOxOxO, xOxOxOx, OxOxOxO' };
121+
const expectTrailers = { '__proto__': null, 'x-foo': 'xOxOxOx, OxOxOxO, xOxOxOx, OxOxOxO' };
124122

125123
assert.deepStrictEqual(res.rawTrailers, expectRawTrailers);
126124
assert.deepStrictEqual(res.trailers, expectTrailers);

test/parallel/test-http-remove-header-stays-removed.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const server = http.createServer(common.mustCall(function(request, response) {
5050
server.listen(0, common.mustCall(function() {
5151
http.get({ port: this.address().port }, common.mustCall((res) => {
5252
assert.strictEqual(res.statusCode, 200);
53-
assert.deepStrictEqual(res.headers, { date: 'coffee o clock' });
53+
assert.deepStrictEqual(res.headers, { __proto__: null, date: 'coffee o clock' });
5454

5555
let response = '';
5656
res.setEncoding('ascii');

0 commit comments

Comments
 (0)