From 82c53822ecef034b1b88f47ebaa13b4d8c8ac836 Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Thu, 18 Apr 2013 03:30:22 +0200 Subject: m_callerid Fix bookkeeping error introduced when unserializing callerid_data This also fixes a memory leak that didn't occur naturally but was triggerable by remote servers Thanks to @SimosNap for the report --- src/modules/m_callerid.cpp | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'src/modules/m_callerid.cpp') diff --git a/src/modules/m_callerid.cpp b/src/modules/m_callerid.cpp index 7f843f252..812df1da0 100644 --- a/src/modules/m_callerid.cpp +++ b/src/modules/m_callerid.cpp @@ -38,26 +38,6 @@ class callerid_data std::list wholistsme; callerid_data() : lastnotify(0) { } - callerid_data(const std::string& str) - { - irc::commasepstream s(str); - std::string tok; - if (s.GetToken(tok)) - { - lastnotify = ConvToInt(tok); - } - while (s.GetToken(tok)) - { - if (tok.empty()) - { - continue; - } - - User *u = ServerInstance->FindNick(tok); - if ((u) && (u->registered == REG_ALL) && (!u->quitting) && (!IS_SERVER(u))) - accepting.insert(u); - } - } std::string ToString(SerializeFormat format) const { @@ -88,8 +68,29 @@ struct CallerIDExtInfo : public ExtensionItem void unserialize(SerializeFormat format, Extensible* container, const std::string& value) { - callerid_data* dat = new callerid_data(value); - set_raw(container, dat); + callerid_data* dat = new callerid_data; + irc::commasepstream s(value); + std::string tok; + if (s.GetToken(tok)) + dat->lastnotify = ConvToInt(tok); + + while (s.GetToken(tok)) + { + if (tok.empty()) + continue; + + User *u = ServerInstance->FindNick(tok); + if ((u) && (u->registered == REG_ALL) && (!u->quitting) && (!IS_SERVER(u))) + { + callerid_data* other = this->get(u, true); + other->wholistsme.push_back(dat); + dat->accepting.insert(u); + } + } + + void* old = set_raw(container, dat); + if (old) + this->free(old); } callerid_data* get(User* user, bool create) -- cgit v1.3.1-10-gc9f91 From 5ca81cf89a1d7cf78a2bb078a25305c1ebab8746 Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Thu, 18 Apr 2013 13:25:28 +0200 Subject: m_callerid Ignore duplicate entries when unserializing callerid_data --- src/modules/m_callerid.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/modules/m_callerid.cpp') diff --git a/src/modules/m_callerid.cpp b/src/modules/m_callerid.cpp index 812df1da0..b0d6b9c64 100644 --- a/src/modules/m_callerid.cpp +++ b/src/modules/m_callerid.cpp @@ -82,9 +82,11 @@ struct CallerIDExtInfo : public ExtensionItem User *u = ServerInstance->FindNick(tok); if ((u) && (u->registered == REG_ALL) && (!u->quitting) && (!IS_SERVER(u))) { - callerid_data* other = this->get(u, true); - other->wholistsme.push_back(dat); - dat->accepting.insert(u); + if (dat->accepting.insert(u).second) + { + callerid_data* other = this->get(u, true); + other->wholistsme.push_back(dat); + } } } -- cgit v1.3.1-10-gc9f91 From f4dda34d06b5f93b28e0ec7d34557199717e147d Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Fri, 19 Apr 2013 17:05:50 +0200 Subject: m_callerid Allow messaging yourself while +g regardless of the ACCEPT list --- src/modules/m_callerid.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/modules/m_callerid.cpp') diff --git a/src/modules/m_callerid.cpp b/src/modules/m_callerid.cpp index b0d6b9c64..4b167f2db 100644 --- a/src/modules/m_callerid.cpp +++ b/src/modules/m_callerid.cpp @@ -362,7 +362,7 @@ public: ModResult PreText(User* user, User* dest, std::string& text) { - if (!dest->IsModeSet('g')) + if (!dest->IsModeSet('g') || (user == dest)) return MOD_RES_PASSTHRU; if (operoverride && IS_OPER(user)) -- cgit v1.3.1-10-gc9f91 From 6e79197210e58d69e7bd512d0b5569c4cb06f3e5 Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Sun, 21 Apr 2013 17:20:28 +0200 Subject: Log some internal errors on DEFAULT loglevel instead of DEBUG, log detected errors in m_callerid --- src/modules/m_callerid.cpp | 11 +++++++++++ src/usermanager.cpp | 6 +++--- src/users.cpp | 8 ++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) (limited to 'src/modules/m_callerid.cpp') diff --git a/src/modules/m_callerid.cpp b/src/modules/m_callerid.cpp index 4b167f2db..37787b525 100644 --- a/src/modules/m_callerid.cpp +++ b/src/modules/m_callerid.cpp @@ -116,11 +116,16 @@ struct CallerIDExtInfo : public ExtensionItem callerid_data *targ = this->get(*it, false); if (!targ) + { + ServerInstance->Logs->Log("m_callerid", DEFAULT, "ERROR: Inconsistency detected in callerid state, please report (1)"); continue; // shouldn't happen, but oh well. + } std::list::iterator it2 = std::find(targ->wholistsme.begin(), targ->wholistsme.end(), dat); if (it2 != targ->wholistsme.end()) targ->wholistsme.erase(it2); + else + ServerInstance->Logs->Log("m_callerid", DEFAULT, "ERROR: Inconsistency detected in callerid state, please report (2)"); } delete dat; } @@ -280,6 +285,7 @@ public: if (!dat2) { // How the fuck is this possible. + ServerInstance->Logs->Log("m_callerid", DEFAULT, "ERROR: Inconsistency detected in callerid state, please report (3)"); return false; } @@ -287,6 +293,9 @@ public: if (it != dat2->wholistsme.end()) // Found me! dat2->wholistsme.erase(it); + else + ServerInstance->Logs->Log("m_callerid", DEFAULT, "ERROR: Inconsistency detected in callerid state, please report (4)"); + user->WriteServ("NOTICE %s :%s is no longer on your accept list", user->nick.c_str(), whotoremove->nick.c_str()); return true; @@ -324,6 +333,8 @@ private: if (it2 != dat->accepting.end()) dat->accepting.erase(it2); + else + ServerInstance->Logs->Log("m_callerid", DEFAULT, "ERROR: Inconsistency detected in callerid state, please report (5)"); } userdata->wholistsme.clear(); diff --git a/src/usermanager.cpp b/src/usermanager.cpp index 2f2eeb6dd..e3ddfc9f2 100644 --- a/src/usermanager.cpp +++ b/src/usermanager.cpp @@ -167,13 +167,13 @@ void UserManager::QuitUser(User *user, const std::string &quitreason, const char { if (user->quitting) { - ServerInstance->Logs->Log("CULLLIST",DEBUG, "*** Warning *** - You tried to quit a user (%s) twice. Did your module call QuitUser twice?", user->nick.c_str()); + ServerInstance->Logs->Log("USERS", DEFAULT, "ERROR: Tried to quit quitting user: " + user->nick); return; } if (IS_SERVER(user)) { - ServerInstance->Logs->Log("CULLLIST",DEBUG, "*** Warning *** - You tried to quit a fake user (%s)", user->nick.c_str()); + ServerInstance->Logs->Log("USERS", DEFAULT, "ERROR: Tried to quit server user: " + user->nick); return; } @@ -239,7 +239,7 @@ void UserManager::QuitUser(User *user, const std::string &quitreason, const char if (iter != this->clientlist->end()) this->clientlist->erase(iter); else - ServerInstance->Logs->Log("USERS", DEBUG, "iter == clientlist->end, can't remove them from hash... problematic.."); + ServerInstance->Logs->Log("USERS", DEFAULT, "ERROR: Nick not found in clientlist, cannot remove: " + user->nick); ServerInstance->Users->uuidlist->erase(user->uuid); } diff --git a/src/users.cpp b/src/users.cpp index dbc3ea444..f48e3642f 100644 --- a/src/users.cpp +++ b/src/users.cpp @@ -546,6 +546,8 @@ CullResult LocalUser::cull() // is only a precaution currently. if (localuseriter != ServerInstance->Users->local_users.end()) ServerInstance->Users->local_users.erase(localuseriter); + else + ServerInstance->Logs->Log("USERS", DEFAULT, "ERROR: LocalUserIter does not point to a valid entry for " + this->nick); ClearInvites(); eh.cull(); @@ -840,6 +842,12 @@ void User::InvalidateCache() bool User::ChangeNick(const std::string& newnick, bool force) { + if (quitting) + { + ServerInstance->Logs->Log("USERS", DEFAULT, "ERROR: Attempted to change nick of a quitting user: " + this->nick); + return false; + } + ModResult MOD_RESULT; if (force) -- cgit v1.3.1-10-gc9f91