v8 crashes hard when setting property remoteAddress on wrapped TLS socket

[coolaj86]

• Version: v6.6.0
• Platform: Ubuntu 16.04.1 LTS Linux ubuntu-512mb-nyc1-01 4.4.0-31-generic path.resolve across drives in Windows #50-Ubuntu SMP Wed Jul 13 00:07:12 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: TLS and/or Stream

When assigning remoteAddress to a tlsSocket that is created from a Duplex, node crashes hard.

I discovered this as part of the workaround for #8752

tlsSocket.remoteAddress = 'x';

aj@ubuntu-512mb-nyc1-01:~/tunnel$ node crash.js
listening on 3001
r undefined
s ::ffff:127.0.0.1
t ::ffff:127.0.0.1
FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
 1: node::Abort() [node]
 2: 0x8d04bc [node]
 3: v8::Utils::ReportApiFailure(char const*, char const*) [node]
 4: node::TLSWrap::OnStreamRead(long, uv_buf_t const&) [node]
 5: node::JSStream::ReadBuffer(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 6: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]
 7: 0xb17d2c [node]
 8: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
 9: 0x6670cd042fd
Aborted (core dumped)

crash.js:

(function () {
  'use strict';

  var net = require('net');
  var tls = require('tls');
  var hostname = 'local.test.ppl.family';
  var tlsOpts = {
    key: require('fs').readFileSync('./privkey.pem')
  , cert: require('fs').readFileSync('./fullchain.pem')
  };
  var Duplex = require('stream').Duplex;
  var WebSocket = require('ws');

  var tls3000 = tls.createServer(tlsOpts, function (tlsSocket) {
    console.log('r', tlsSocket.remoteAddress);
    console.log('s', tlsSocket._handle._parentWrap._handle.owner.stream.remoteAddress);
    tlsSocket._remoteAddress = tlsSocket._handle._parentWrap._handle.owner.stream.remoteAddress;
    console.log('t', tlsSocket._remoteAddress);
    tlsSocket.remoteAddress = 'x'; // causes core dump
    console.log('u', tlsSocket.remoteAddress);
  });

  var Dup = {
    write: function (chunk, encoding, cb) {
      //console.log('_write', chunk.byteLength);
      this.__my_socket.write(chunk, encoding);
      cb();
    }
  , read: function (size) {
      //console.log('_read');
      var x = this.__my_socket.read(size);
      if (x) {
        console.log('_read', size);
        this.push(x);
      }
    }
  };

  function connectHttps(servername, socket) {
    var myDuplex = new Duplex();

    myDuplex.__my_socket = socket;
    myDuplex._write = Dup.write;
    myDuplex._read = Dup.read;
    myDuplex.remoteFamily = socket.remoteFamily;
    myDuplex.remoteAddress = socket.remoteAddress;
    myDuplex.remotePort = socket.remotePort;
    myDuplex.localFamily = socket.localFamily;
    myDuplex.localAddress = socket.localAddress;
    myDuplex.localPort = socket.localPort;

    tls3000.emit('connection', myDuplex);

    socket.on('data', function (chunk) {
      myDuplex.push(chunk);
    });
  }

  var tcp3000 = net.createServer();
  tcp3000.listen(3001, function () {
    console.log('listening on 3001');

    var wstunneler = new WebSocket('wss://' + hostname + ':3001/?access_token=xyz', { rejectUnauthorized: false });
    wstunneler.on('open', function () {
      console.log('ws open');
    });
  });

  tcp3000.on('connection', function (socket) {
    socket.once('data', function (firstChunk) {
      connectHttps(null, socket);
      socket.unshift(firstChunk);
    });

  });
}());

It may be possible to slim this down a little further and still get it to crash if #8752 gets fixed... except that the remoteAddress property would probably show up as expected and then I wouldn't be trying to write to it in the first place...

[mscdex]

/cc @indutny

[coolaj86]

I also get the same FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal. error when doing this (same situation as above, but without setting the remateAddress variable either):

tls.createServer(tunnelAdminTlsOpts, function (tlsSocket) {
    ( thing.notDefined || thing.notDefinedEither ).oopsThisWontBeDefined(tlsSocket);
});

Which seems very much like it should throw an error in JS, not in V8.

Sidenote:

For reference, this is where the remote address is, if anyone needs it:

tlsSocket._handle._parent.owner.stream.remoteAddress

[Trott]

@nodejs/crypto

[coolaj86]

I've tested in node v9.11.1 and this is still an issue.

If you'd like to get some ssl certs to test with you can click-click-next through the process here: https://greenlock.ppl.family/

[Trott]

I've tested in node v9.11.1 and this is still an issue.

@coolaj86 Would it be easy enough to test with 10.0.0? (9.x goes EOL in a couple months, but 10.x will be supported for another 2.5 years.)

[coolaj86]

Yep, still a problem in 10.0.0

r undefined
s ::ffff:127.0.0.1
t ::ffff:127.0.0.1
node[4922]: ../src/node.cc:1379:void node::ReportException(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Message>): Assertion `!message.IsEmpty()' failed.
 1: node::Abort() [node]
 2: 0x87b6c5 [node]
 3: 0x87c41a [node]
 4: node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [node]
 5: node::FatalException(v8::Isolate*, v8::TryCatch const&) [node]
 6: node::JSStream::DoWrite(node::WriteWrap*, uv_buf_t*, unsigned long, uv_stream_s*) [node]
 7: node::TLSWrap::EncOut() [node]
 8: node::TLSWrap::OnStreamRead(long, uv_buf_t const&) [node]
 9: node::JSStream::ReadBuffer(v8::FunctionCallbackInfo<v8::Value> const&) [node]
10: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [node]
11: 0xad62fa [node]
12: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
13: 0x3e9fce0427d
Aborted (core dumped)