fixes issues found by static analysis tools

[DonatJR]

@bencikpeter There were some issues found in the tokenLib as well, here are all issues which I fixed in this PR:

There were also two issues I did not yet fix (as I wasn't sure how to proceed):

As you can see the handles returned from OpenProcess are not closed if hasSeCreateTokenPrivilege or processIsLocalSystem throw an exception. I was not sure if you did not catch those on purpose. Any suggestions on the easiest way to deal with these issues? Do you want to do it yourself? (Maybe they are not even relevant anymore after #91?)

[SailReal]

Wooooooow, nice work 🍩!
Thanks a lot, I have nothing to add 🎉.

Will approve it as soon as we finished discussing what we do (fixing or ignore with reason) with the remaining two issues in the tokenlib.

[bencikpeter]

@DonatJR I´ll have a look at those issues and push directly into this PR if I may 😉

[DonatJR]

@bencikpeter Sure, go ahead

[bencikpeter]

@DonatJR could you please send me a full report? (I am not a HTWG student anymore, so no VPN 😞). I don´t quite understand what exactly the error is trying to say... I don´t throw any exeptions manually with hasPrilivege functions and WinAPI is C-oriented and does not throw as well.. that leads me to believe that only exceptions that can occur are runtime (like allocation error) but in that case, resoruce leaks should be reported here as well, since if constructor throws, destructor is not called and memory leak happens... but Coverity ignores it so it might mean something else? 🤔

tokenTemplate::tokenTemplate(HANDLE &userToken) {

	//load internal NtCreateToken function
	HMODULE hModule = LoadLibrary(L"ntdll.dll");
	NtCreateToken = (NT_CREATE_TOKEN)GetProcAddress(hModule, "NtCreateToken");

	//parse token
	DWORD bufferSize = 0;
	GetTokenInformation(userToken, TokenType, NULL, 0, &bufferSize);
	SetLastError(0);
	GetTokenInformation(userToken, TokenType, (LPVOID)&tokenType, bufferSize, &bufferSize);
	if (GetLastError() != 0)
	{
		throw TokenParsingException();
	}

	bufferSize = 0;
	GetTokenInformation(userToken, TokenUser, NULL, 0, &bufferSize);
	SetLastError(0);
	tokenUser = (PTOKEN_USER) new BYTE[bufferSize];
	GetTokenInformation(userToken, TokenUser, (LPVOID)tokenUser, bufferSize, &bufferSize);
	if (GetLastError() != 0)
	{
		throw TokenParsingException();
	}

[bencikpeter]

Since probable nature of exceptions will be runtime and I believe should not be handled directly inside the function, to address the resource leak, this should suffice:

	for (auto const& processPid : processes)
	{
		HANDLE processHandle;
		if ((processHandle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processPid)) == NULL) continue;
		try
		{
			if (processIsLocalSystem(processHandle))
			{
				return processHandle;
			}
		}
		catch (const std::exception&)
		{
			CloseHandle(processHandle);
			throw;
		}
		CloseHandle(processHandle);
	}

[DonatJR]

@bencikpeter Unfortunately, I don't know how to get a pdf report from the website based static analysis tool these errors are from, sorry. :/

The exception probably stems from a call to std::cout with an integer-value. Your solution to free'ing the handle (and stopping the resource leak) looks good to me, I would have handled it the same way.

[bencikpeter]

Looks good to me 👍