fix: "invalid ip address" "daemon can be crashed by remote user" #96
Conversation
TomCoded
force-pushed
the
fix-93
branch
2 times, most recently
from
b458077
to
9370155
Compare
There was a problem hiding this comment.
Thanks for hunting this down and for the PR @TomCoded! It looks like the core issue is multiaddr not getting the information it needs. For resiliency I think it might be better to wrap the internals of getMultiaddr with a try-catch and then repeat your failed socket logic if the returned addr doesn't exist. That way if the remote data is still bad from multiaddr's point of view, we're still handling the error.
If you could add some tests around the getMultiaddr function showing the catch working, that would be very appreciated.
TomCoded
force-pushed
the
fix-93
branch
2 times, most recently
from
a4d7fe1
to
6010e59
Compare
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: libp2p#93 fixes: ipfs/js-ipfs#1447
|
Happy to help @jacobheun! Changed to a try-catch block in get-multiaddr, which should catch any other errors that result in a socket multiaddr can't handle. |
|
Looks good! Thanks again @TomCoded! I'll get a release out shortly. |
Hello maintainers! I saw the remote vulnerability from js-ipfs #1447
and hunted it down. Are there any problems with or possible
improvements to this proposed solution? I believe it will also resolve
js-libp2p-tcp issue #93
Cheers,
Tom
--
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 terminates processing of a destroyed socket before multiaddr
causes the crash.
fixes: #93
fixes: ipfs/js-ipfs#1447