Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix: invalid ip address and daemon can be crashed by remote user
Browse files Browse the repository at this point in the history
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: #93
fixes: ipfs/js-ipfs#1447
  • Loading branch information
TomCoded authored and Jacob Heun committed Jul 31, 2018
1 parent 6c36a46 commit 4b04b17
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 14 deletions.
33 changes: 19 additions & 14 deletions src/get-multiaddr.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions src/listener.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
54 changes: 54 additions & 0 deletions test/get-multiaddr.spec.js
Original file line number Diff line number Diff line change
@@ -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()
})
})

0 comments on commit 4b04b17

Please sign in to comment.