m_pNetwork from OnUnknownUserRaw is not to be trusted
authorKyle Fuller <redacted>
Fri, 30 Aug 2013 06:15:17 +0000 (07:15 +0100)
committerKyle Fuller <redacted>
Fri, 30 Aug 2013 13:15:39 +0000 (14:15 +0100)
Don't rely on m_pNetwork it seems to store another user in the
OnUnknownUserRaw ZNC hook and not NULL as expected.

In certain circumstances, the client will send a palaver negotiation
before the registration to ZNC has finished. If this happens,
m_pNetwork value from the module hook could contain a different network
instead of NULL.

If m_pNetwork has another user's network, it would cause this device to be
attached to a different network.

This bug could manifest in a different way, if the network is deleted in the
meantime (once m_pNetwork is set to a network that is deleted). When this
certain code path is run again, it could cause a crash because we would try and
read from memory which has been deallocated.

You may need to delete moddata/palaver/palaver.conf to lose any incorrect
config which was caused by this bug.

palaver.cpp

index 0f735a1b7e3bc8a9366b9d6ff0a195338b0c400c..670681a0f74308c63d779ff27e7ca7721500fc6d 100644 (file)
@@ -562,8 +562,9 @@ public:
 
                                device.AddClient(*pClient);
 
-                               if (m_pNetwork) {
-                                       if (device.AddNetwork(*m_pNetwork) && device.InNegotiation() == false) {
+                               CIRCNetwork *pNetwork = pClient->GetNetwork();
+                               if (pNetwork) {
+                                       if (device.AddNetwork(*pNetwork) && device.InNegotiation() == false) {
                                                Save();
                                        }
                                }
git clone https://git.99rst.org/PROJECT