From 81841f76c863844210baf991d5c8cdfd01e57353 Mon Sep 17 00:00:00 2001 From: Peter Powell Date: Sun, 22 Feb 2015 20:24:31 +0000 Subject: [PATCH] Fix a minor crash when a user has no class after OnCheckReady. This bug looks serious but it can only be triggered with a very unusual server configuration problem. If you haven't already had a crash then you probably aren't at any risk. The way this crash happens is: 1. InspIRCd::DoBackgroundUserStuff is called by the main loop. 2. In the switch statement curr->registered is set to REG_NICKUSER so InspIRCd::AllModulesReportReady is called. 3. InspIRCd::AllModulesReportReady calls the OnCheckReady event in m_cgiirc. 4. m_cgiirc calls RecheckClass which sets the user's class to NULL and calls LocalUser::SetClass followed by LocalUser::CheckClass. 5. The user doesn't match any classes in LocalUser::SetClass so LocalUser::CheckClass quits the user with with "Access denied by configuration". 6. Control flow returns to InspIRCd::DoBackgroundUserStuff when InspIRCd::AllModulesReportReady returns false. 7. The if statement at the end of InspIRCd::DoBackgroundUserStuff calls ConnectClass::GetRegTimeout on curr->MyClass. 8. ConnectClass::GetRegTimeout tries to access a member of this which is NULL. 9. The server crashes with a SEGFAULT. --- src/userprocess.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/userprocess.cpp b/src/userprocess.cpp index 0ef95e4136..69c31f8400 100644 --- a/src/userprocess.cpp +++ b/src/userprocess.cpp @@ -104,10 +104,15 @@ void InspIRCd::DoBackgroundUserStuff() curr->FullConnect(); continue; } + + // If the user has been quit in OnCheckReady then we shouldn't + // quit them again for having a registration timeout. + if (curr->quitting) + continue; break; } - if (curr->registered != REG_ALL && (Time() > (curr->signon + curr->MyClass->GetRegTimeout()))) + if (curr->registered != REG_ALL && curr->MyClass && (Time() > (curr->signon + curr->MyClass->GetRegTimeout()))) { /* * registration timeout -- didnt send USER/NICK/HOST