Skip to content

Commit e78bf55

Browse files
authored
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: nodejs#61897 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 09c21d8 commit e78bf55

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
@@ -55,6 +55,8 @@ const {
5555
kUniqueHeaders,
5656
parseUniqueHeadersOption,
5757
OutgoingMessage,
58+
validateHeaderName,
59+
validateHeaderValue,
5860
} = require('_http_outgoing');
5961
const {
6062
kOutHeaders,
@@ -333,13 +335,20 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) {
333335
return;
334336
}
335337

338+
if (checkInvalidHeaderChar(link)) {
339+
throw new ERR_INVALID_CHAR('header content', 'Link');
340+
}
341+
336342
head += 'Link: ' + link + '\r\n';
337343

338344
const keys = ObjectKeys(hints);
339345
for (let i = 0; i < keys.length; i++) {
340346
const key = keys[i];
341347
if (key !== 'link') {
342-
head += key + ': ' + hints[key] + '\r\n';
348+
validateHeaderName(key);
349+
const value = hints[key];
350+
validateHeaderValue(key, value);
351+
head += key + ': ' + value + '\r\n';
343352
}
344353
}
345354

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)