zeromq / netmq

A 100% native C# implementation of ZeroMQ for .NET
Other
2.95k stars 743 forks source link

ArgumentOutOfRangeException while processing handshake causes process exit #1049

Closed mnmr closed 1 year ago

mnmr commented 1 year ago

Environment

NetMQ Version:    4.0.1.10
Operating System: Windows 11 x64
.NET Version:     7.0.103

Expected behaviour

Ability for NetMQ to handle/catch errors, and/or provide an error handling mechanism that users can plug in to. It is not nice to have code in your process that can cause it to abrubtly exit.

Actual behaviour

The process terminates.

2023-03-06 00:44:24.880 [FTL] Host Caught unhandled app "ArgumentOutOfRangeException" in "api", "UNABLE TO PREVENT PROCESS EXIT!"
System.ArgumentOutOfRangeException: Index and count must refer to a location within the buffer. (Parameter 'bytes')
   at System.Text.ASCIIEncoding.GetString(Byte[] bytes, Int32 byteIndex, Int32 byteCount)
   at NetMQ.Msg.GetString(Encoding encoding, Int32 offset, Int32 count)
   at NetMQ.Core.Mechanisms.Mechanism.IsCommand(String command, Msg& msg)
   at NetMQ.Core.Mechanisms.NullMechanism.ProcessHandshakeCommand(Msg& msg)
   at NetMQ.Core.Transports.StreamEngine.ProcessHandshakeCommand(Msg& msg)
   at NetMQ.Core.Transports.StreamEngine.ProcessInput()
   at NetMQ.Core.Transports.StreamEngine.Handle(Action action, SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.Transports.StreamEngine.FeedAction(Action action, SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.Transports.StreamEngine.InCompleted(SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.IOObject.InCompleted(SocketError socketError, Int32 bytesTransferred)
   at NetMQ.Core.Utils.Proactor.Loop()
   at System.Threading.Thread.StartCallback()

Steps to reproduce the behaviour

I'm not sure, but someone (this is an API server where each client has their own set of DEALER/XPUB sockets to connect to) is connecting to the port and sending data that NetMQ interprets as a handshake command. The code to determine whether the Msg is a command seems to make invalid assumptions about the Data in the Msg and fails when it tries to get the command name.

Because the entire call-chain is without exception handler, the exception is only caught by the AppDomain.CurrentDomain.UnhandledException += AppOnUnobservedException; handler, after which the process terminates.

mnmr commented 1 year ago

The obvious workaround for this issue would be to add an additional while loop with a try-catch block in the Proactor.Loop method to catch any internal errors and only surface those that are truly process-crash-worthy. This would mitigate the issue significantly even if the bug remained.

We've been using this in production for 5+ years and are only now seeing this issue, so it's absolutely caused by some specific bytes arriving at the socket.