Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some minor hygiene issues on resource management #1

Open
JohnLaTwC opened this issue Sep 13, 2021 · 0 comments
Open

Some minor hygiene issues on resource management #1

JohnLaTwC opened this issue Sep 13, 2021 · 0 comments

Comments

@JohnLaTwC
Copy link

These are mostly hygiene issues than actual bugs. Resource allocations will be freed at process exit anyway. But the code takes care to release resources and so just noting them. Code may be copy/pasted into other frameworks where resource hygiene may matter more.

1. The call to CreateBindingHandle initializes bHandle. The CleanUp routine should call RpcBindingFree on this resource.

Allocated here:

rStatus = CreateBindingHandle(lpwDomainname, lpwUsername, lpwPassword, lpwTarget, &bHandle);

and here:
rStatus = CreateBindingHandle(lpwDomainname, lpwUsername, lpwPassword, lpwTarget, &bHandle);

2. Should check for failed allocation before writing:

	container_info.Level = 2;
!	container_info.DriverInfo.Level2 = (DRIVER_INFO_2*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(DRIVER_INFO_2));   /// check for failed allocation
	container_info.DriverInfo.Level2->cVersion = 3;

This is both in the reflective and non-reflective cases:

container_info.DriverInfo.Level2 = (DRIVER_INFO_2*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(DRIVER_INFO_2));

container_info.DriverInfo.Level2 = (DRIVER_INFO_2*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(DRIVER_INFO_2));

3. Should free memory from call to Utf8ToUtf16 in CleanUp

4. Code inconsistently switches from MAX_BUF to MAX_PATH

LPWSTR GetDriverPath(IN LPWSTR lpwTarget) {
...
	WCHAR wcKeyName[MAX_BUF] = { 0 };
	DWORD dwNamelen = MAX_BUF;
...
		if (lResult == 0) {
			for (DWORD i = 0; ; i++) {
				RtlZeroMemory(wcKeyName, sizeof(wcKeyName));
				lResult = RegEnumKeyEx(hSubKeyHandle, i, wcKeyName, &dwNamelen, NULL, NULL, NULL, NULL);
				if (StrStrIW(wcKeyName, L"ntprint.inf_amd64")) {
					wcscpy_s(lpwDriverPath, MAX_BUF, L"C:\\Windows\\System32\\DriverStore\\FileRepository\\");
					wcscat_s(lpwDriverPath, MAX_BUF, wcKeyName);
					wcscat_s(lpwDriverPath, MAX_BUF, L"\\Amd64\\UNIDRV.DLL");
					wprintf(L"[>] pDriverPath:  -> %s\n\n", lpwDriverPath);
					break;
				}

				if (lResult == ERROR_NO_MORE_ITEMS) {
					break;
				}

!				dwNamelen = MAX_PATH;  //changed from MAX_BUF to MAX_PATH?
			}
		}
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant