mirror of
https://github.com/znc/znc.git
synced 2026-07-05 01:11:53 +02:00
Fix memory leak and null dereference in CZNC::LoadUsers
Before this commit, when pUser->SetBeingDeleted(true) is executed, pUser is an empty unique_ptr, because release() was already called on it. Therefore, pUser->SetBeingDeleted is unidefined behaviour. Also, AddUser only takes ownership of the passed user pointer if it succeeds. In case of a failure, it's the caller's responsibility to delete the user. Fix this by keeping a raw pointer to the user, and handling it accordingly when AddUser fails. I have no idea whether SetBeingDeleted is necessary there, leaving it just in case. Maybe it would be better if we could change the semantics of AddUser to always take ownership of the pointer, or even take unique_ptr, but I have no idea how to adapt Python bindings in modpython to such change.
This commit is contained in:
+4
-5
@@ -1275,13 +1275,12 @@ bool CZNC::LoadUsers(CConfig& config, CString& sError) {
|
||||
}
|
||||
|
||||
CString sErr;
|
||||
if (!AddUser(pUser.release(), sErr, true)) {
|
||||
CUser* pRawUser = pUser.release();
|
||||
if (!AddUser(pRawUser, sErr, true)) {
|
||||
sError = "Invalid user [" + sUserName + "] " + sErr;
|
||||
}
|
||||
|
||||
if (!sError.empty()) {
|
||||
CUtils::PrintError(sError);
|
||||
pUser->SetBeingDeleted(true);
|
||||
pRawUser->SetBeingDeleted(true);
|
||||
delete pRawUser;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user