Disconnecting from an IRC server sends multiple QUIT messages #762

New Issue

2026-10-05T04:43:16+02:00

adam commented

2026-10-05 04:43:16 +02:00

When a user disconnects ZNC from an IRC server (i.e. runs /znc disconnect or /msg *status disconnect), ZNC attempts to send 3 quit messages to the remote server, and only the first one contains the quit message. Here's the debug log after I run /znc disconnect a quit message:

[2017-04-05 21:31:46.010616] (elyscape/Freenode) CLI -> ZNC [ZNC disconnect a quit message]
[2017-04-05 21:31:46.010765] (elyscape/Freenode) ZNC -> IRC [QUIT :a quit message]
[2017-04-05 21:31:46.010946] (elyscape/Freenode) ZNC -> IRC [QUIT :ZNC 1.6.4 - http://znc.in]
[2017-04-05 21:31:46.010989] (elyscape/Freenode) ZNC -> CLI [:*status!znc@znc.in PRIVMSG Elyscape :Disconnected from IRC. Use 'connect' to reconnect.]
[2017-04-05 21:31:46.011117] IRC::elyscape::Freenode == Disconnected()
[2017-04-05 21:31:46.011159] (elyscape/Freenode) ZNC -> CLI [:Elyscape!~Elyscape@unaffiliated/elyscape MODE Elyscape :-i]
[2017-04-05 21:31:46.011264] (elyscape/Freenode) ZNC -> IRC [QUIT :ZNC 1.6.4 - http://znc.in]

Here's what's going on. If ZNC is connected to a remote server and receives a disconnect command from a client:

ZNC calls Quit(sQuitMsg) on the socket and then SetIRCConnectEnabled(false) on the network.
- Quit() on the socket writes the quit message to the underlying socket and then marks it for closing after all data is written.
- SetIRCConnectEnabled() on the network checks IsConnected() on the socket and, if true, calls Quit() on it. This will always occur, because the socket hasn't had time to finish writing the quit message.
  - Quit() writes a default quit message to the socket.
The CSocketManager destroys the socket object.
- The CIRCSocket destructor is executed and it calls Quit() again.
  - Quit() writes a default quit message to the socket.
- The superclass destructors are executed, finally closing the socket file descriptors.

If a user disables a network in the settings or webadmin, the same thing happens, only the process starts at the call to SetIRCConnectEnabled(), so only 2 quit messages are sent.

This has a few ramifications:

In theory, a poorly-coded IRC server could try to process the extra quit messages. Depending on the specifics, this could result in a crash, including the extra quit messages in the quit message, double-free of a user object, or something else.
More importantly, it means that catching and examining user quit messages in a module with OnSendToIRC() is unreliable and contains false positives.

The good news is that the fix is quite simple: we just need to add a check to the beginning of CIRCSock::Quit() that returns if IsClosed() returns true. I'll make a pull request shortly.

When a user disconnects ZNC from an IRC server (i.e. runs `/znc disconnect` or `/msg *status disconnect`), ZNC attempts to send 3 quit messages to the remote server, and only the first one contains the quit message. Here's the debug log after I run `/znc disconnect a quit message`: ``` [2017-04-05 21:31:46.010616] (elyscape/Freenode) CLI -> ZNC [ZNC disconnect a quit message] [2017-04-05 21:31:46.010765] (elyscape/Freenode) ZNC -> IRC [QUIT :a quit message] [2017-04-05 21:31:46.010946] (elyscape/Freenode) ZNC -> IRC [QUIT :ZNC 1.6.4 - http://znc.in] [2017-04-05 21:31:46.010989] (elyscape/Freenode) ZNC -> CLI [:*status!znc@znc.in PRIVMSG Elyscape :Disconnected from IRC. Use 'connect' to reconnect.] [2017-04-05 21:31:46.011117] IRC::elyscape::Freenode == Disconnected() [2017-04-05 21:31:46.011159] (elyscape/Freenode) ZNC -> CLI [:Elyscape!~Elyscape@unaffiliated/elyscape MODE Elyscape :-i] [2017-04-05 21:31:46.011264] (elyscape/Freenode) ZNC -> IRC [QUIT :ZNC 1.6.4 - http://znc.in] ``` Here's what's going on. If ZNC is connected to a remote server and receives a disconnect command from a client: - [ZNC calls `Quit(sQuitMsg)` on the socket and then `SetIRCConnectEnabled(false)` on the network][disconnect]. - `Quit()` on the socket [writes the quit message to the underlying socket and then marks it for closing after all data is written][quit]. - `SetIRCConnectEnabled()` on the network [checks `IsConnected()` on the socket and, if true, calls `Quit()` on it][setircconnectenabled]. This will always occur, because the socket hasn't had time to finish writing the quit message. - `Quit()` writes a default quit message to the socket. - The CSocketManager [destroys the socket object][csocketmanager]. - The CIRCSocket destructor is executed and it [calls `Quit()` again][circsocket-destructor]. - `Quit()` writes a default quit message to the socket. - The superclass destructors are executed, [finally closing the socket file descriptors][csock-destructor]. If a user disables a network in the settings or webadmin, the same thing happens, only the process starts at the call to `SetIRCConnectEnabled()`, so only 2 quit messages are sent. This has a few ramifications: 1. In theory, a poorly-coded IRC server could try to process the extra quit messages. Depending on the specifics, this could result in a crash, including the extra quit messages in the quit message, double-free of a user object, or something else. 2. More importantly, it means that catching and examining user quit messages in a module with `OnSendToIRC()` is unreliable and contains false positives. The good news is that the fix is quite simple: we just need to add a check to the beginning of `CIRCSock::Quit()` that returns if `IsClosed()` returns true. I'll make a pull request shortly. [disconnect]: https://github.com/znc/znc/blob/znc-1.6.5/src/ClientCommand.cpp#L357-L360 [quit]: https://github.com/znc/znc/blob/znc-1.6.5/src/IRCSock.cpp#L124-L129 [setircconnectenabled]: https://github.com/znc/znc/blob/znc-1.6.5/src/IRCNetwork.cpp#L1282-L1284 [csocketmanager]: https://github.com/jimloco/Csocket/blob/448e18a29ed383451db3cb648a72da4fcbb3f8e5/Csocket.cc#L3621 [circsocket-destructor]: https://github.com/znc/znc/blob/znc-1.6.5/src/IRCSock.cpp#L113 [csock-destructor]: https://github.com/jimloco/Csocket/blob/448e18a29ed383451db3cb648a72da4fcbb3f8e5/Csocket.cc#L990

adam closed this issue

2026-10-05 04:43:16 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: adam/znc#762