Skip to content

Commit dae6eef

Browse files
rsclarkemarco-ippolito
authored andcommitted
http: validate headers in writeEarlyHints
Add validateHeaderName/validateHeaderValue checks for non-link headers and checkInvalidHeaderChar for the Link value in HTTP/1.1 writeEarlyHints, closing a CRLF injection gap where header names and values were concatenated into the raw response without validation. Also tighten linkValueRegExp to reject CR/LF inside the <...> URL portion of Link header values. PR-URL: #61897 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 42bd0e2 commit dae6eef

3 files changed

Lines changed: 52 additions & 2 deletions

File tree

lib/_http_server.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ const {
5252
kUniqueHeaders,
5353
parseUniqueHeadersOption,
5454
OutgoingMessage,
55+
validateHeaderName,
56+
validateHeaderValue,
5557
} = require('_http_outgoing');
5658
const {
5759
kOutHeaders,
@@ -328,13 +330,20 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) {
328330
return;
329331
}
330332

333+
if (checkInvalidHeaderChar(link)) {
334+
throw new ERR_INVALID_CHAR('header content', 'Link');
335+
}
336+
331337
head += 'Link: ' + link + '\r\n';
332338

333339
const keys = ObjectKeys(hints);
334340
for (let i = 0; i < keys.length; i++) {
335341
const key = keys[i];
336342
if (key !== 'link') {
337-
head += key + ': ' + hints[key] + '\r\n';
343+
validateHeaderName(key);
344+
const value = hints[key];
345+
validateHeaderValue(key, value);
346+
head += key + ': ' + value + '\r\n';
338347
}
339348
}
340349

lib/internal/validators.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ function validateUnion(value, name, union) {
517517
(not necessarily a valid URI reference) followed by zero or more
518518
link-params separated by semicolons.
519519
*/
520-
const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/;
520+
const linkValueRegExp = /^(?:<[^>\r\n]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/;
521521

522522
/**
523523
* @param {any} value

test/parallel/test-http-early-hints-invalid-argument.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,44 @@ const testResBody = 'response content\n';
4747
req.on('information', common.mustNotCall());
4848
}));
4949
}
50+
51+
{
52+
const server = http.createServer(common.mustCall((req, res) => {
53+
debug('Server sending early hints with CRLF injection...');
54+
55+
assert.throws(() => {
56+
res.writeEarlyHints({
57+
'link': '</styles.css>; rel=preload; as=style',
58+
'X-Custom': 'valid\r\nSet-Cookie: session=evil',
59+
});
60+
}, (err) => err.code === 'ERR_INVALID_CHAR');
61+
62+
assert.throws(() => {
63+
res.writeEarlyHints({
64+
'link': '</styles.css>; rel=preload; as=style',
65+
'X-Custom\r\nSet-Cookie: session=evil': 'value',
66+
});
67+
}, (err) => err.code === 'ERR_INVALID_HTTP_TOKEN');
68+
69+
assert.throws(() => {
70+
res.writeEarlyHints({
71+
link: '</styles.css\r\nSet-Cookie: session=evil>; rel=preload; as=style',
72+
});
73+
}, (err) => err.code === 'ERR_INVALID_ARG_VALUE');
74+
75+
debug('Server sending full response...');
76+
res.end(testResBody);
77+
server.close();
78+
}));
79+
80+
server.listen(0, common.mustCall(() => {
81+
const req = http.request({
82+
port: server.address().port, path: '/'
83+
});
84+
85+
req.end();
86+
debug('Client sending request...');
87+
88+
req.on('information', common.mustNotCall());
89+
}));
90+
}

0 commit comments

Comments
 (0)