186171 - Fixed memory leaks in passhook.dll

3 files changed, 43 insertions, 12 deletions

diff --git a/ldap/synctools/passwordsync/passhand.cpp b/ldap/synctools/passwordsync/passhand.cpp
index 7d5af3cd..ddad1977 100644
--- a/ldap/synctools/passwordsync/passhand.cpp
+++ b/ldap/synctools/passwordsync/passhand.cpp
@@ -200,11 +200,17 @@ exit:
 
 int clearSet(PASS_INFO_LIST* passInfoList)
 {
-	// ToDo: zero out memory
+	while (!passInfoList->empty()) {
+		PASS_INFO& pi = passInfoList->back();
+		SecureZeroMemory(pi.password, strlen(pi.password));
+		free(pi.password);
+		free(pi.username);
+		passInfoList->pop_back();
+	}
 
 	passInfoList->clear();
 
-	return -1;
+	return 0;
 }
 
 int encrypt(char* plainTextBuf, int plainTextLen, char* cipherTextBuf, int cipherTextLen, int* resultTextLen)
diff --git a/ldap/synctools/passwordsync/passhook/passhook.cpp b/ldap/synctools/passwordsync/passhook/passhook.cpp
index f059c538..eba277a2 100644
--- a/ldap/synctools/passwordsync/passhook/passhook.cpp
+++ b/ldap/synctools/passwordsync/passhook/passhook.cpp
@@ -50,8 +50,6 @@
 
 NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId, PUNICODE_STRING Password)
 {
-	char singleByteUsername[PASSHAND_BUF_SIZE];
-	char singleBytePassword[PASSHAND_BUF_SIZE];
 	HANDLE passhookEventHandle = OpenEvent(EVENT_MODIFY_STATE, FALSE, PASSHAND_EVENT_NAME);
 	PASS_INFO newPassInfo;
 	PASS_INFO_LIST passInfoList;
@@ -78,18 +76,34 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId,
 	}
 	RegCloseKey(regKey);
 
-	_snprintf(singleByteUsername, PASSHAND_BUF_SIZE, "%S", UserName->Buffer);
-	singleByteUsername[UserName->Length / 2] = '\0';
-	_snprintf(singleBytePassword, PASSHAND_BUF_SIZE, "%S", Password->Buffer);
-	singleBytePassword[Password->Length / 2] = '\0';
+	// This memory will be free'd by calling clearSet below
+	newPassInfo.username = (char*)malloc((UserName->Length / 2) + 1);
+	newPassInfo.password = (char*)malloc((Password->Length / 2) + 1);
+
+	if (newPassInfo.username && newPassInfo.password) {
+		_snprintf(newPassInfo.username, (UserName->Length / 2), "%S", UserName->Buffer);
+		_snprintf(newPassInfo.password, (Password->Length / 2), "%S", Password->Buffer);
+		newPassInfo.username[UserName->Length / 2] = '\0';
+		newPassInfo.password[Password->Length / 2] = '\0';
+	} else {
+		if(outLog.is_open()) {
+			timeStamp(&outLog);
+			outLog << "failed to allocate memory for username and password" << endl;
+		}
+		free(newPassInfo.username);
+		free(newPassInfo.password);
+		goto exit;
+	}
 
 	if(outLog.is_open())
 	{
 		timeStamp(&outLog);
-		outLog << "user " << singleByteUsername << " password changed" << endl;
-		//outLog << "user " << singleByteUsername << " password changed to " << singleBytePassword << endl;
+		outLog << "user " << newPassInfo.username << " password changed" << endl;
+		//outLog << "user " << newPassInfo.username << " password changed to " << newPassInfo.password << endl;
 	}
 
+	// loadSet allocates memory for the usernames and password.  We need to be
+	// sure to free it by calling clearSet.
 	if(loadSet(&passInfoList, "passhook.dat") == 0)
 	{
 		if(outLog.is_open())
@@ -107,10 +121,10 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId,
 		}
 	}
 
-	newPassInfo.username = singleByteUsername;
-	newPassInfo.password = singleBytePassword;
+	// Add the new change to the list
 	passInfoList.push_back(newPassInfo);
 
+	// Save the list to disk
 	if(saveSet(&passInfoList, "passhook.dat") == 0)
 	{
 		if(outLog.is_open())
@@ -128,6 +142,10 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId,
 		}
 	}
 
+	// We need to call clearSet so memory gets free'd
+	clearSet(&passInfoList);
+
+exit:
 	if(passhookEventHandle == NULL)
 	{
 		if(outLog.is_open())
@@ -140,6 +158,7 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId,
 	else
 	{
 		SetEvent(passhookEventHandle);
+		CloseHandle(passhookEventHandle);
 	}
 
 	outLog.close();
diff --git a/ldap/synctools/passwordsync/passsync/syncserv.cpp b/ldap/synctools/passwordsync/passsync/syncserv.cpp
index 86de672e..63bb451e 100644
--- a/ldap/synctools/passwordsync/passsync/syncserv.cpp
+++ b/ldap/synctools/passwordsync/passsync/syncserv.cpp
@@ -375,6 +375,9 @@ int PassSyncService::SyncPasswords()
 				ldap_memfree(dn);
 				dn = NULL;
 
+				// zero out memory used for password
+				SecureZeroMemory(tempPassInfo->password, strlen(tempPassInfo->password));
+
 				// free the username and password
 				free(tempPassInfo->username);
 				free(tempPassInfo->password);
@@ -658,6 +661,9 @@ void PassSyncService::UpdateBackoff()
 			tempPassInfo = currentPassInfo;
 			currentPassInfo++;
 
+			// zero out memory used for password
+			SecureZeroMemory(tempPassInfo->password, strlen(tempPassInfo->password));
+
 			// free the username and password
 			free(tempPassInfo->username);
 			free(tempPassInfo->password);