Skip to content

Commit 16f4d0e

Browse files
committed
Ported from OpenSUSE:nodejs8-8.17.0-lp152.147.1:CVE-2019-15606.patch Original commit message: commit 2eee90e Author: Sam Roberts <vieuxtech@gmail.com> Date: Fri Jan 10 15:00:11 2020 -0800 http: strip trailing OWS from header values HTTP header values can have trailing OWS, but it should be stripped. It is not semantically part of the header's value, and if treated as part of the value, it can cause spurious inequality between expected and actual header values. Note that a single SPC of leading OWS is common before the field-value, and it is already handled by the HTTP parser by stripping all leading OWS. It is only the trailing OWS that must be stripped by the parser user. header-field = field-name ":" OWS field-value OWS ; https://tools.ietf.org/html/rfc7230#section-3.2 OWS = *( SP / HTAB ) ; https://tools.ietf.org/html/rfc7230#section-3.2.3 Fixes: https://hackerone.com/reports/730779 PR-URL: https://github.com/nodejs-private/node-private/pull/191 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
1 parent 05c74fb commit 16f4d0e

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

src/node_http_parser.cc

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ const uint32_t kOnMessageComplete = 3;
7474
const uint32_t kOnExecute = 4;
7575

7676

77+
inline bool IsOWS(char c) {
78+
return c == ' ' || c == '\t';
79+
}
80+
7781
// helper class for the Parser
7882
struct StringPtr {
7983
StringPtr() {
@@ -133,13 +137,22 @@ struct StringPtr {
133137

134138

135139
Local<String> ToString(Environment* env) const {
136-
if (str_)
140+
if (size_ != 0)
137141
return OneByteString(env->isolate(), str_, size_);
138142
else
139143
return String::Empty(env->isolate());
140144
}
141145

142146

147+
// Strip trailing OWS (SPC or HTAB) from string.
148+
Local<String> ToTrimmedString(Environment* env) {
149+
while (size_ > 0 && IsOWS(str_[size_ - 1])) {
150+
size_--;
151+
}
152+
return ToString(env);
153+
}
154+
155+
143156
const char* str_;
144157
bool on_heap_;
145158
size_t size_;
@@ -685,7 +698,7 @@ class Parser : public AsyncWrap {
685698
size_t j = 0;
686699
while (i < num_values_ && j < arraysize(argv) / 2) {
687700
argv[j * 2] = fields_[i].ToString(env());
688-
argv[j * 2 + 1] = values_[i].ToString(env());
701+
argv[j * 2 + 1] = values_[i].ToTrimmedString(env());
689702
i++;
690703
j++;
691704
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// This test ensures that the http-parser strips leading and trailing OWS from
5+
// header values. It sends the header values in chunks to force the parser to
6+
// build the string up through multiple calls to on_header_value().
7+
8+
const assert = require('assert');
9+
const http = require('http');
10+
const net = require('net');
11+
12+
function check(hdr, snd, rcv) {
13+
const server = http.createServer(common.mustCall((req, res) => {
14+
assert.strictEqual(req.headers[hdr], rcv);
15+
req.pipe(res);
16+
}));
17+
18+
server.listen(0, common.mustCall(function() {
19+
const client = net.connect(this.address().port, start);
20+
function start() {
21+
client.write('GET / HTTP/1.1\r\n' + hdr + ':', drain);
22+
}
23+
24+
function drain() {
25+
if (snd.length === 0) {
26+
return client.write('\r\nConnection: close\r\n\r\n');
27+
}
28+
client.write(snd.shift(), drain);
29+
}
30+
31+
const bufs = [];
32+
client.on('data', function(chunk) {
33+
bufs.push(chunk);
34+
});
35+
client.on('end', common.mustCall(function() {
36+
const head = Buffer.concat(bufs)
37+
.toString('latin1')
38+
.split('\r\n')[0];
39+
assert.strictEqual(head, 'HTTP/1.1 200 OK');
40+
server.close();
41+
}));
42+
}));
43+
}
44+
45+
check('host', [' \t foo.com\t'], 'foo.com');
46+
check('host', [' \t foo\tcom\t'], 'foo\tcom');
47+
check('host', [' \t', ' ', ' foo.com\t', '\t '], 'foo.com');
48+
check('host', [' \t', ' \t'.repeat(100), '\t '], '');
49+
check('host', [' \t', ' - - - - ', '\t '], '- - - -');

0 commit comments

Comments
 (0)