From 4fb70be963aef7d07630396eff0bdf65e536d2ea Mon Sep 17 00:00:00 2001
From: TomCoded <tomtinkerer@gmail.com>
Date: Mon, 23 Jul 2018 03:48:26 +0000
Subject: [PATCH] fix: invalid ip address and daemon can be crashed by remote
 user

Per the nodeJS documentation, a Net socket.remoteAddress value may
be undefined if the socket is destroyed, as by a client disconnect.
A multiaddr cannot be created for an invalid IP address (such as
the undefined remote address of a destroyed socket). Currently
the attempt results in a crash that can be triggered remotely. This
commit catches the exception in get-multiaddr and returns an
undefined value to listener rather than throwing an exception when
trying to process defective or destroyed socket data. Listener then
terminates processing of the incoming p2p connections that generate
this error condition.

fixes: https://github.com/libp2p/js-libp2p-tcp/issues/93
fixes: https://github.com/ipfs/js-ipfs/issues/1447
---
 src/get-multiaddr.js       | 33 +++++++++++++----------
 src/listener.js            |  9 +++++++
 test/get-multiaddr.spec.js | 54 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 14 deletions(-)
 create mode 100644 test/get-multiaddr.spec.js

diff --git a/src/get-multiaddr.js b/src/get-multiaddr.js
index bc99799..4be7243 100644
--- a/src/get-multiaddr.js
+++ b/src/get-multiaddr.js
@@ -2,27 +2,32 @@
 
 const multiaddr = require('multiaddr')
 const Address6 = require('ip-address').Address6
+const debug = require('debug')
+const log = debug('libp2p:tcp:get-multiaddr')
 
 module.exports = (socket) => {
   let ma
 
-  if (socket.remoteFamily === 'IPv6') {
-    const addr = new Address6(socket.remoteAddress)
+  try {
+    if (socket.remoteFamily === 'IPv6') {
+      const addr = new Address6(socket.remoteAddress)
 
-    if (addr.v4) {
-      const ip4 = addr.to4().correctForm()
-      ma = multiaddr('/ip4/' + ip4 +
-        '/tcp/' + socket.remotePort
-      )
+      if (addr.v4) {
+        const ip4 = addr.to4().correctForm()
+        ma = multiaddr('/ip4/' + ip4 +
+          '/tcp/' + socket.remotePort
+        )
+      } else {
+        ma = multiaddr('/ip6/' + socket.remoteAddress +
+          '/tcp/' + socket.remotePort
+        )
+      }
     } else {
-      ma = multiaddr('/ip6/' + socket.remoteAddress +
-        '/tcp/' + socket.remotePort
-      )
+      ma = multiaddr('/ip4/' + socket.remoteAddress +
+        '/tcp/' + socket.remotePort)
     }
-  } else {
-    ma = multiaddr('/ip4/' + socket.remoteAddress +
-      '/tcp/' + socket.remotePort)
+  } catch (err) {
+    log(err)
   }
-
   return ma
 }
diff --git a/src/listener.js b/src/listener.js
index f04b6e5..30b3076 100644
--- a/src/listener.js
+++ b/src/listener.js
@@ -25,6 +25,15 @@ module.exports = (handler) => {
     socket.on('error', noop)
 
     const addr = getMultiaddr(socket)
+    if (!addr) {
+      if (socket.remoteAddress === undefined) {
+        log('connection closed before p2p connection made')
+      } else {
+        log('error interpreting incoming p2p connection')
+      }
+      return
+    }
+
     log('new connection', addr.toString())
 
     const s = toPull.duplex(socket)
diff --git a/test/get-multiaddr.spec.js b/test/get-multiaddr.spec.js
new file mode 100644
index 0000000..d7199ac
--- /dev/null
+++ b/test/get-multiaddr.spec.js
@@ -0,0 +1,54 @@
+/* eslint-env mocha */
+'use strict'
+
+const chai = require('chai')
+const dirtyChai = require('dirty-chai')
+const expect = chai.expect
+chai.use(dirtyChai)
+const getMultiaddr = require('../src/get-multiaddr')
+
+const goodSocket4 = {
+  remoteAddress: '127.0.0.1',
+  remotePort: '9090',
+  remoteFamily: 'IPv4'
+}
+
+const goodSocket6 = {
+  remoteAddress: '::1',
+  remotePort: '9090',
+  remoteFamily: 'IPv6'
+}
+
+const badSocket = {}
+
+const badSocketData = {
+  remoteAddress: 'aewmrn4awoew',
+  remotePort: '234',
+  remoteFamily: 'Hufflepuff'
+}
+
+describe('getMultiaddr multiaddr creation', () => {
+  it('creates multiaddr from valid socket data', (done) => {
+    expect(getMultiaddr(goodSocket4))
+      .to.exist()
+    done()
+  })
+
+  it('creates multiaddr from valid IPv6 socket data', (done) => {
+    expect(getMultiaddr(goodSocket6))
+      .to.exist()
+    done()
+  })
+
+  it('returns undefined multiaddr from missing socket data', (done) => {
+    expect(getMultiaddr(badSocket))
+      .to.equal(undefined)
+    done()
+  })
+
+  it('returns undefined multiaddr from unparseable socket data', (done) => {
+    expect(getMultiaddr(badSocketData))
+      .to.equal(undefined)
+    done()
+  })
+})