From: Alexey Sokolov Date: Sat, 5 Sep 2026 23:09:25 +0100 Subject: Fix crash while unloading modules The bug allowed unprivileged users to trigger use-after-free while unloading modules Note that the fix requires a slight API change for OnEmbeddedWebRequest: now the callers (webadmin is the only known caller of it) call it via DoEmbeddedWebRequest() instead. The modules which implement this hook need no changes. CVE-2026-82373 Found by Claude from Anthropic, reported by Ada Logics. --- Upstream: https:\/\/github.com\/znc\/znc\/commit\/4b88c64c6e8df29c73568ae1c8b35d797bfa509a CVE: CVE-2026-82373 Signed-off-by: Titouan Christophe --- include\/znc\/Modules.h | 71 ++++++++++++++++++++- include\/znc\/main.h | 30 ++------- modules\/modperl\/functions.in | 2 +- modules\/modpython\/functions.in | 2 +- modules\/webadmin.cpp | 12 ++-- src\/Client.cpp | 29 +++------ src\/Modules.cpp | 112 +++++++++++++++------------------ 7 files changed, 145 insertions(+), 113 deletions(-) diff --git a\/include\/znc\/Modules.h b\/include\/znc\/Modules.h index ec8b36f883..d842390675 100644 --- a\/include\/znc\/Modules.h +++ b\/include\/znc\/Modules.h @@ -465,6 +465,9 @@ class CModule { void SetNetwork(CIRCNetwork* pNetwork); void SetClient(CClient* pClient); + \/** True if the module can be safely unloaded. *\/ + bool IsCallStackEmpty() const; + \/** This function throws CModule::UNLOAD which causes this module to be unloaded. *\/ void Unload() { throw UNLOAD; } @@ -535,9 +538,11 @@ class CModule { * @return The List. *\/ virtual VWebSubPages& GetSubPages() { return m_vSubPages; } + private: \/** Using this hook, module can embed web stuff directly to different places. - * This method is called whenever embededded modules I\/O happens. + * This method is called whenever embedded modules I\/O happens. * Name of used .tmpl file (if any) is up to caller. + * Callers call it via `DoEmbeddedWebRequest()`. * @param WebSock Socket for web connection, don't do bad things with it. * @param sPageName Describes the place where web stuff is embedded to. * @param Tmpl Template. Depending on context, you can do various stuff with it. @@ -547,6 +552,9 @@ class CModule { virtual bool OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl); + public: + bool DoEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, + CTemplate& Tmpl); \/** Called just before znc.conf is rehashed *\/ virtual void OnPreRehash(); @@ -1528,6 +1536,9 @@ class CModule { m_mssRegistry; \/\/!< way to save name\/value pairs. Note there is no encryption involved in this VWebSubPages m_vSubPages; std::map m_mCommands; + int m_iCallStackDepth = 0; + + friend struct CModCallProtector; }; class CModules : public std::vector, private CCoreTranslationMixin { @@ -1766,4 +1777,62 @@ class CModules : public std::vector, private CCoreTranslationMixin { CClient* m_pClient; }; +struct CModCallProtector { + explicit CModCallProtector(CModule& pMod) : m_pMod(&pMod) { + pMod.m_iCallStackDepth++; + } + ~CModCallProtector() { m_pMod->m_iCallStackDepth--; } + + CModule* m_pMod; +}; + +#ifndef SWIG +template +struct CTemporaryModField { + CTemporaryModField(CModule& pMod, T* pValue) { + m_pMod = &pMod; + m_pOldT = (pMod.*Getter)(); + if (pValue) { + (pMod.*Setter)(pValue); + } + } + + ~CTemporaryModField() { (m_pMod->*Setter)(m_pOldT); } + + CModule* m_pMod; + T* m_pOldT; +}; + +using CTemporaryModClient = + CTemporaryModField; +using CTemporaryModNetwork = + CTemporaryModField; +using CTemporaryModUser = + CTemporaryModField; + +\/\/ Same as above, but for plural CModules +template +struct CTemporaryModsField { + CTemporaryModsField(CModules& pMods, T* pValue) { + m_pMods = &pMods; + m_pOldT = (pMods.*Getter)(); + if (pValue) { + (pMods.*Setter)(pValue); + } + } + + ~CTemporaryModsField() { (m_pMods->*Setter)(m_pOldT); } + + CModules* m_pMods; + T* m_pOldT; +}; + +using CTemporaryModsClient = + CTemporaryModsField; +using CTemporaryModsNetwork = + CTemporaryModsField; +using CTemporaryModsUser = + CTemporaryModsField; +#endif + #endif \/\/ !ZNC_MODULES_H diff --git a\/include\/znc\/main.h b\/include\/znc\/main.h index 7d22fd43d4..28924d2c02 100644 --- a\/include\/znc\/main.h +++ b\/include\/znc\/main.h @@ -56,21 +56,12 @@ extern bool ZNC_NO_NEED_TO_DO_ANYTHING_ON_MODULE_CALL_EXITER; #define _GLOBALMODULECALL(macFUNC, macUSER, macNETWORK, macCLIENT, macEXITER) \ do { \ CModules& GMods = CZNC::Get().GetModules(); \ - CUser* pOldGUser = GMods.GetUser(); \ - CIRCNetwork* pOldGNetwork = GMods.GetNetwork(); \ - CClient* pOldGClient = GMods.GetClient(); \ - GMods.SetUser(macUSER); \ - GMods.SetNetwork(macNETWORK); \ - GMods.SetClient(macCLIENT); \ + CTemporaryModsUser TempUser(GMods, macUSER); \ + CTemporaryModsNetwork TempNetwork(GMods, macNETWORK); \ + CTemporaryModsClient TempClient(GMods, macCLIENT); \ if (GMods.macFUNC) { \ - GMods.SetUser(pOldGUser); \ - GMods.SetNetwork(pOldGNetwork); \ - GMods.SetClient(pOldGClient); \ *macEXITER = true; \ } \ - GMods.SetUser(pOldGUser); \ - GMods.SetNetwork(pOldGNetwork); \ - GMods.SetClient(pOldGClient); \ } while (false) #define _USERMODULECALL(macFUNC, macUSER, macNETWORK, macCLIENT, macEXITER) \ @@ -84,17 +75,11 @@ extern bool ZNC_NO_NEED_TO_DO_ANYTHING_ON_MODULE_CALL_EXITER; } \ if (macUSER != nullptr) { \ CModules& UMods = macUSER->GetModules(); \ - CIRCNetwork* pOldUNetwork = UMods.GetNetwork(); \ - CClient* pOldUClient = UMods.GetClient(); \ - UMods.SetNetwork(macNETWORK); \ - UMods.SetClient(macCLIENT); \ + CTemporaryModsNetwork TempNetwork(UMods, macNETWORK); \ + CTemporaryModsClient TempClient(UMods, macCLIENT); \ if (UMods.macFUNC) { \ - UMods.SetNetwork(pOldUNetwork); \ - UMods.SetClient(pOldUClient); \ *macEXITER = true; \ } \ - UMods.SetNetwork(pOldUNetwork); \ - UMods.SetClient(pOldUClient); \ } \ } while (false) @@ -109,13 +94,10 @@ extern bool ZNC_NO_NEED_TO_DO_ANYTHING_ON_MODULE_CALL_EXITER; } \ if (macNETWORK != nullptr) { \ CModules& NMods = macNETWORK->GetModules(); \ - CClient* pOldNClient = NMods.GetClient(); \ - NMods.SetClient(macCLIENT); \ + CTemporaryModsClient TempClient(NMods, macCLIENT); \ if (NMods.macFUNC) { \ - NMods.SetClient(pOldNClient); \ *macEXITER = true; \ } \ - NMods.SetClient(pOldNClient); \ } \ } while (false) diff --git a\/modules\/modperl\/functions.in b\/modules\/modperl\/functions.in index a44c818a62..bfa671191c 100644 --- a\/modules\/modperl\/functions.in +++ b\/modules\/modperl\/functions.in @@ -66,7 +66,7 @@ void OnServerCapResult(const CString& sCap, bool bSuccess) void OnClientAttached() void OnClientDetached() EModRet OnTimerAutoJoin(CChan& Channel) -bool OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl) +bool OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl)=false EModRet OnAddNetwork(CIRCNetwork& Network, CString& sErrorRet) EModRet OnDeleteNetwork(CIRCNetwork& Network) EModRet OnSendToClient(CString& sLine, CClient& Client) diff --git a\/modules\/modpython\/functions.in b\/modules\/modpython\/functions.in index 76ed437a2c..d2914f9dad 100644 --- a\/modules\/modpython\/functions.in +++ b\/modules\/modpython\/functions.in @@ -66,7 +66,7 @@ void OnServerCapResult(const CString& sCap, bool bSuccess) void OnClientAttached() void OnClientDetached() EModRet OnTimerAutoJoin(CChan& Channel) -bool OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl) +bool OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl)=false EModRet OnAddNetwork(CIRCNetwork& Network, CString& sErrorRet) EModRet OnDeleteNetwork(CIRCNetwork& Network) EModRet OnSendToClient(CString& sLine, CClient& Client) diff --git a\/modules\/webadmin.cpp b\/modules\/webadmin.cpp index 0b72413813..ac77517c11 100644 --- a\/modules\/webadmin.cpp +++ b\/modules\/webadmin.cpp @@ -742,7 +742,7 @@ class CWebAdminMod : public CModule { CTemplate& mod = Tmpl.AddRow("EmbeddedModuleLoop"); mod.insert(Tmpl.begin(), Tmpl.end()); mod["WebadminAction"] = "display"; - if ((*i)->OnEmbeddedWebRequest(WebSock, "webadmin\/channel", + if ((*i)->DoEmbeddedWebRequest(WebSock, "webadmin\/channel", mod)) { mod["Embed"] = WebSock.FindTmpl(*i, "WebadminChan.tmpl"); mod["ModName"] = (*i)->GetModName(); @@ -816,7 +816,7 @@ class CWebAdminMod : public CModule { TmplMod["ChanName"] = pChan->GetName(); TmplMod["WebadminAction"] = "change"; FOR_EACH_MODULE(it, pNetwork) { - (*it)->OnEmbeddedWebRequest(WebSock, "webadmin\/channel", TmplMod); + (*it)->DoEmbeddedWebRequest(WebSock, "webadmin\/channel", TmplMod); } if (!CZNC::Get().WriteConfig()) { @@ -1027,7 +1027,7 @@ class CWebAdminMod : public CModule { CTemplate& mod = Tmpl.AddRow("EmbeddedModuleLoop"); mod.insert(Tmpl.begin(), Tmpl.end()); mod["WebadminAction"] = "display"; - if ((*i)->OnEmbeddedWebRequest(WebSock, "webadmin\/network", + if ((*i)->DoEmbeddedWebRequest(WebSock, "webadmin\/network", mod)) { mod["Embed"] = WebSock.FindTmpl(*i, "WebadminNetwork.tmpl"); mod["ModName"] = (*i)->GetModName(); @@ -1246,7 +1246,7 @@ class CWebAdminMod : public CModule { TmplMod["Name"] = pNetwork->GetName(); TmplMod["WebadminAction"] = "change"; FOR_EACH_MODULE(it, make_pair(pUser, pNetwork)) { - (*it)->OnEmbeddedWebRequest(WebSock, "webadmin\/network", TmplMod); + (*it)->DoEmbeddedWebRequest(WebSock, "webadmin\/network", TmplMod); } if (!CZNC::Get().WriteConfig()) { @@ -1670,7 +1670,7 @@ class CWebAdminMod : public CModule { CTemplate& mod = Tmpl.AddRow("EmbeddedModuleLoop"); mod.insert(Tmpl.begin(), Tmpl.end()); mod["WebadminAction"] = "display"; - if ((*i)->OnEmbeddedWebRequest(WebSock, "webadmin\/user", mod)) { + if ((*i)->DoEmbeddedWebRequest(WebSock, "webadmin\/user", mod)) { mod["Embed"] = WebSock.FindTmpl(*i, "WebadminUser.tmpl"); mod["ModName"] = (*i)->GetModName(); } @@ -1731,7 +1731,7 @@ class CWebAdminMod : public CModule { TmplMod["Username"] = sUsername; TmplMod["WebadminAction"] = "change"; FOR_EACH_MODULE(it, pUser) { - (*it)->OnEmbeddedWebRequest(WebSock, "webadmin\/user", TmplMod); + (*it)->DoEmbeddedWebRequest(WebSock, "webadmin\/user", TmplMod); } if (!CZNC::Get().WriteConfig()) { diff --git a\/src\/Client.cpp b\/src\/Client.cpp index 0f60f9a11c..db1ec7a62e 100644 --- a\/src\/Client.cpp +++ b\/src\/Client.cpp @@ -21,8 +21,8 @@ #include #include -using std::set; using std::map; +using std::set; using std::vector; #define CALLMOD(MOD, CLIENT, USER, NETWORK, FUNC) \ @@ -30,10 +30,9 @@ using std::vector; CModule* pModule = nullptr; \ if (NETWORK && (pModule = (NETWORK)->GetModules().FindModule(MOD))) { \ try { \ - CClient* pOldClient = pModule->GetClient(); \ - pModule->SetClient(CLIENT); \ + CModCallProtector Inside(*pModule); \ + CTemporaryModClient TempClient(*pModule, CLIENT); \ pModule->FUNC; \ - pModule->SetClient(pOldClient); \ } catch (const CModule::EModException& e) { \ if (e == CModule::UNLOAD) { \ (NETWORK)->GetModules().UnloadModule(MOD); \ @@ -41,13 +40,10 @@ using std::vector; } \ } else if ((pModule = (USER)->GetModules().FindModule(MOD))) { \ try { \ - CClient* pOldClient = pModule->GetClient(); \ - CIRCNetwork* pOldNetwork = pModule->GetNetwork(); \ - pModule->SetClient(CLIENT); \ - pModule->SetNetwork(NETWORK); \ + CModCallProtector Inside(*pModule); \ + CTemporaryModClient TempClient(*pModule, CLIENT); \ + CTemporaryModNetwork TempNetwork(*pModule, NETWORK); \ pModule->FUNC; \ - pModule->SetClient(pOldClient); \ - pModule->SetNetwork(pOldNetwork); \ } catch (const CModule::EModException& e) { \ if (e == CModule::UNLOAD) { \ (USER)->GetModules().UnloadModule(MOD); \ @@ -55,16 +51,11 @@ using std::vector; } \ } else if ((pModule = CZNC::Get().GetModules().FindModule(MOD))) { \ try { \ - CClient* pOldClient = pModule->GetClient(); \ - CIRCNetwork* pOldNetwork = pModule->GetNetwork(); \ - CUser* pOldUser = pModule->GetUser(); \ - pModule->SetClient(CLIENT); \ - pModule->SetNetwork(NETWORK); \ - pModule->SetUser(USER); \ + CModCallProtector Inside(*pModule); \ + CTemporaryModClient TempClient(*pModule, CLIENT); \ + CTemporaryModNetwork TempNetwork(*pModule, NETWORK); \ + CTemporaryModUser TempUser(*pModule, USER); \ pModule->FUNC; \ - pModule->SetClient(pOldClient); \ - pModule->SetNetwork(pOldNetwork); \ - pModule->SetUser(pOldUser); \ } catch (const CModule::EModException& e) { \ if (e == CModule::UNLOAD) { \ CZNC::Get().GetModules().UnloadModule(MOD); \ diff --git a\/src\/Modules.cpp b\/src\/Modules.cpp index d553cbb794..2c4c056fa1 100644 --- a\/src\/Modules.cpp +++ b\/src\/Modules.cpp @@ -36,67 +36,45 @@ bool ZNC_NO_NEED_TO_DO_ANYTHING_ON_MODULE_CALL_EXITER; #warning "your crap box doesn't define RTLD_LOCAL !?" #endif -#define MODUNLOADCHK(func) \ - for (CModule * pMod : *this) { \ - try { \ - CClient* pOldClient = pMod->GetClient(); \ - pMod->SetClient(m_pClient); \ - CUser* pOldUser = nullptr; \ - if (m_pUser) { \ - pOldUser = pMod->GetUser(); \ - pMod->SetUser(m_pUser); \ - } \ - CIRCNetwork* pNetwork = nullptr; \ - if (m_pNetwork) { \ - pNetwork = pMod->GetNetwork(); \ - pMod->SetNetwork(m_pNetwork); \ - } \ - pMod->func; \ - if (m_pUser) pMod->SetUser(pOldUser); \ - if (m_pNetwork) pMod->SetNetwork(pNetwork); \ - pMod->SetClient(pOldClient); \ - } catch (const CModule::EModException& e) { \ - if (e == CModule::UNLOAD) { \ - UnloadModule(pMod->GetModName()); \ - } \ - } \ - } - -#define MODHALTCHK(func) \ - bool bHaltCore = false; \ - for (CModule * pMod : *this) { \ - try { \ - CModule::EModRet e = CModule::CONTINUE; \ - CClient* pOldClient = pMod->GetClient(); \ - pMod->SetClient(m_pClient); \ - CUser* pOldUser = nullptr; \ - if (m_pUser) { \ - pOldUser = pMod->GetUser(); \ - pMod->SetUser(m_pUser); \ - } \ - CIRCNetwork* pNetwork = nullptr; \ - if (m_pNetwork) { \ - pNetwork = pMod->GetNetwork(); \ - pMod->SetNetwork(m_pNetwork); \ - } \ - e = pMod->func; \ - if (m_pUser) pMod->SetUser(pOldUser); \ - if (m_pNetwork) pMod->SetNetwork(pNetwork); \ - pMod->SetClient(pOldClient); \ - if (e == CModule::HALTMODS) { \ - break; \ - } else if (e == CModule::HALTCORE) { \ - bHaltCore = true; \ - } else if (e == CModule::HALT) { \ - bHaltCore = true; \ - break; \ - } \ - } catch (const CModule::EModException& e) { \ - if (e == CModule::UNLOAD) { \ - UnloadModule(pMod->GetModName()); \ - } \ - } \ - } \ +#define MODUNLOADCHK(func) \ + for (CModule* pMod : *this) { \ + try { \ + CModCallProtector inside(*pMod); \ + CTemporaryModClient TempClient(*pMod, m_pClient); \ + CTemporaryModUser TempUser(*pMod, m_pUser); \ + CTemporaryModNetwork TempNetwork(*pMod, m_pNetwork); \ + pMod->func; \ + } catch (const CModule::EModException& e) { \ + if (e == CModule::UNLOAD) { \ + UnloadModule(pMod->GetModName()); \ + } \ + } \ + } + +#define MODHALTCHK(func) \ + bool bHaltCore = false; \ + for (CModule* pMod : *this) { \ + try { \ + CModCallProtector inside(*pMod); \ + CModule::EModRet e = CModule::CONTINUE; \ + CTemporaryModClient TempClient(*pMod, m_pClient); \ + CTemporaryModUser TempUser(*pMod, m_pUser); \ + CTemporaryModNetwork TempNetwork(*pMod, m_pNetwork); \ + e = pMod->func; \ + if (e == CModule::HALTMODS) { \ + break; \ + } else if (e == CModule::HALTCORE) { \ + bHaltCore = true; \ + } else if (e == CModule::HALT) { \ + bHaltCore = true; \ + break; \ + } \ + } catch (const CModule::EModException& e) { \ + if (e == CModule::UNLOAD) { \ + UnloadModule(pMod->GetModName()); \ + } \ + } \ + } \ return bHaltCore; \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/ Timer \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/ @@ -205,6 +183,8 @@ void CModule::SetUser(CUser* pUser) { m_pUser = pUser; } void CModule::SetNetwork(CIRCNetwork* pNetwork) { m_pNetwork = pNetwork; } void CModule::SetClient(CClient* pClient) { m_pClient = pClient; } +bool CModule::IsCallStackEmpty() const { return m_iCallStackDepth == 0; } + CString CModule::ExpandString(const CString& sStr) const { CString sRet; return ExpandString(sStr, sRet); @@ -622,6 +602,11 @@ bool CModule::OnEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, CTemplate& Tmpl) { return false; } +bool CModule::DoEmbeddedWebRequest(CWebSock& WebSock, const CString& sPageName, + CTemplate& Tmpl) { + CModCallProtector inside(*this); + return OnEmbeddedWebRequest(WebSock, sPageName, Tmpl); +} \/\/ !Webmods bool CModule::OnLoad(const CString& sArgs, CString& sMessage) { @@ -1953,6 +1938,11 @@ bool CModules::UnloadModule(const CString& sModule, CString& sRetMsg) { return false; } + if (!pModule->IsCallStackEmpty()) { + sRetMsg = t_f("Module [{1}] is being called, cannot unload.")(sMod); + return false; + } + bool bSuccess; bool bHandled = false; _GLOBALMODULECALL(OnModuleUnloading(pModule, bSuccess, sRetMsg),

Other Versions#

Patches#

Vulnerabilities#