Skip to content

Commit dafdc0a

Browse files
rsclarkerichardlau
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 aee2a18 commit dafdc0a

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

lib/_http_server.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ const {
5353
kUniqueHeaders,
5454
parseUniqueHeadersOption,
5555
OutgoingMessage,
56+
validateHeaderName,
57+
validateHeaderValue,
5658
} = require('_http_outgoing');
5759
const {
5860
kOutHeaders,
@@ -331,13 +333,20 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) {
331333
return;
332334
}
333335

336+
if (checkInvalidHeaderChar(link)) {
337+
throw new ERR_INVALID_CHAR('header content', 'Link');
338+
}
339+
334340
head += 'Link: ' + link + '\r\n';
335341

336342
const keys = ObjectKeys(hints);
337343
for (let i = 0; i < keys.length; i++) {
338344
const key = keys[i];
339345
if (key !== 'link') {
340-
head += key + ': ' + hints[key] + '\r\n';
346+
validateHeaderName(key);
347+
const value = hints[key];
348+
validateHeaderValue(key, value);
349+
head += key + ': ' + value + '\r\n';
341350
}
342351
}
343352

lib/internal/validators.js

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

514514
/**
515515
* @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)