File CVE-2019-15606.patch of Package nodejs8.20728

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2019-15606.patch of Package nodejs8.20728

commit 2eee90e959ca4abaf53caf238d063c396f2ea17c
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>

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index c2cd7a213b..420e94564e 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -74,6 +74,10 @@ const uint32_t kOnMessageComplete = 3;
 const uint32_t kOnExecute = 4;
 
 
+inline bool IsOWS(char c) {
+  return c == ' ' || c == '\t';
+}
+
 // helper class for the Parser
 struct StringPtr {
   StringPtr() {
@@ -133,13 +137,22 @@ struct StringPtr {
 
 
   Local<String> ToString(Environment* env) const {
-    if (str_)
+    if (size_ != 0)
       return OneByteString(env->isolate(), str_, size_);
     else
       return String::Empty(env->isolate());
   }
 
 
+  // Strip trailing OWS (SPC or HTAB) from string.
+  Local<String> ToTrimmedString(Environment* env) {
+    while (size_ > 0 && IsOWS(str_[size_ - 1])) {
+      size_--;
+    }
+    return ToString(env);
+  }
+
+
   const char* str_;
   bool on_heap_;
   size_t size_;
@@ -669,7 +682,7 @@ class Parser : public AsyncWrap, public StreamListener {
       size_t j = 0;
       while (i < num_values_ && j < arraysize(argv) / 2) {
         argv[j * 2] = fields_[i].ToString(env());
-        argv[j * 2 + 1] = values_[i].ToString(env());
+        argv[j * 2 + 1] = values_[i].ToTrimmedString(env());
         i++;
         j++;
       }
diff --git a/test/parallel/test-http-header-owstext.js b/test/parallel/test-http-header-owstext.js
new file mode 100644
index 0000000000..bc094137a2
--- /dev/null
+++ b/test/parallel/test-http-header-owstext.js
@@ -0,0 +1,49 @@
+'use strict';
+const common = require('../common');
+
+// This test ensures that the http-parser strips leading and trailing OWS from
+// header values. It sends the header values in chunks to force the parser to
+// build the string up through multiple calls to on_header_value().
+
+const assert = require('assert');
+const http = require('http');
+const net = require('net');
+
+function check(hdr, snd, rcv) {
+  const server = http.createServer(common.mustCall((req, res) => {
+    assert.strictEqual(req.headers[hdr], rcv);
+    req.pipe(res);
+  }));
+
+  server.listen(0, common.mustCall(function() {
+    const client = net.connect(this.address().port, start);
+    function start() {
+      client.write('GET / HTTP/1.1\r\n' + hdr + ':', drain);
+    }
+
+    function drain() {
+      if (snd.length === 0) {
+        return client.write('\r\nConnection: close\r\n\r\n');
+      }
+      client.write(snd.shift(), drain);
+    }
+
+    const bufs = [];
+    client.on('data', function(chunk) {
+      bufs.push(chunk);
+    });
+    client.on('end', common.mustCall(function() {
+      const head = Buffer.concat(bufs)
+        .toString('latin1')
+        .split('\r\n')[0];
+      assert.strictEqual(head, 'HTTP/1.1 200 OK');
+      server.close();
+    }));
+  }));
+}
+
+check('host', [' \t foo.com\t'], 'foo.com');
+check('host', [' \t foo\tcom\t'], 'foo\tcom');
+check('host', [' \t', ' ', ' foo.com\t', '\t '], 'foo.com');
+check('host', [' \t', ' \t'.repeat(100), '\t '], '');
+check('host', [' \t', ' - - - -   ', '\t '], '- - - -');

Places

File CVE-2019-15606.patch of Package nodejs8.20728

Places