Mike Mike - 3 months ago 25
C++ Question

PUNICODE_STRING printed to a log

I have a password filter dll which, for now at least, is simply going to write to a log the username and the password when a user account's password is successfully changed. I'm coming up to some age-old problems with c-strings.

The two variables I'm interested in are of the type PUNICODE_STRING (which is simply a *UNICODE_STRING). Naturally I search the web for this data type which landed me here: http://msdn.microsoft.com/en-us/library/windows/desktop/aa380518(v=vs.85).aspx

Give that information I know that I can access the length of this animal and that it has a pointer to a wide-character string. Awesome. That is plenty of knowledge to extract the contents I'll need (so I thought).

Here is my dll code (dllmain.cpp)

// dllmain.cpp : Defines the entry point for the DLL application.
#include "stdafx.h"
#include <iostream>
#define LOGFILE "c:\\PasswordFilter.txt"

BOOL APIENTRY DllMain( HMODULE hModule,
DWORD ul_reason_for_call,
LPVOID lpReserved
)
{
switch (ul_reason_for_call)
{
case DLL_PROCESS_ATTACH:
case DLL_THREAD_ATTACH:
case DLL_THREAD_DETACH:
case DLL_PROCESS_DETACH:
break;
}
return TRUE;
}

void WriteToLog (const wchar_t* UserName, const wchar_t* NewPassword)
{
#ifdef LOGFILE
FILE* log = fopen(LOGFILE, "a");
if(NULL == log)
{
return;
}
fwprintf(log,L"%s password was successfully reset to %s\r\n",UserName,NewPassword);
fclose(log);
#endif
return;
}

LPWSTR __stdcall ConvertWideChar(PUNICODE_STRING input)
{
wchar_t* wszString = new wchar_t[input->Length + 1];
memset(wszString, 0, sizeof(wchar_t) * (input->Length + 1));
wcsncpy(wszString,input->Buffer,input->Length);

#ifdef LOGFILE
FILE* log = fopen(LOGFILE, "a");
if(NULL != log)
{
fwprintf(log,L"value of wszString: %s\r\n",wszString);
fclose(log);
}
#endif

return wszString;
}

BOOLEAN __stdcall InitializeChangeNotify(void)
{
return TRUE;
}

NTSTATUS __stdcall PasswordChangeNotify(
PUNICODE_STRING UserName,
ULONG RelativeId,
PUNICODE_STRING NewPassword
)
{
LPWSTR ConvertedUserName = ConvertWideChar(UserName);
LPWSTR ConvertedNewPassword = ConvertWideChar(NewPassword);
WriteToLog(ConvertedUserName,ConvertedNewPassword);
return 0;
}

BOOLEAN __stdcall PasswordFilter(
PUNICODE_STRING AccountName,
PUNICODE_STRING FullName,
PUNICODE_STRING Password,
BOOLEAN SetOperation
)
{
return TRUE;
}


And here is the contents of the header file (stdafx.h)

// stdafx.h : include file for standard system include files,
// or project specific include files that are used frequently, but
// are changed infrequently
//

#pragma once

#include "targetver.h"

#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers
// Windows Header Files:
#include <stdio.h>
#include <windows.h>
#include <winnt.h>
#include <NTSecAPI.h>

// TODO: reference additional headers your program requires here


Naturally my concern is to retain accuracy. And this is where my pain begins. We have a wchar_t which communicated to me that this is likely in unicode with a UTF16 format (like all other windows based strings). The code above copies the PUNICODE_STRING's buffer to a zero-terminated wchar_t (since the buffer isn't guaranteed to be zero terminated itself) and then attempts to output that string to a log file.

I first attempted to use WideCharToMultiByte as a way to copy the buffer to a character string but that never seemed to work. The result would be null each and every time. This also ran the issue of translating unicode values to a simpler format which runs the risk of data loss. So I decided to go with what I have there for now, but the logged values have ?'s and other garbage after them, which is likely a unicode problem.

I would REALLY like to retain the unicode encoding type of this data structure if possible. How can I do so and print my values to a log?

Answer

You are not using the UNICODE_STRING::Length field correctly. It is expressed in bytes, not in characters. So you are not allocating+filling your null-terminated buffers correctly. You are also leaking the buffers you allocate.

You don't need to allocate the buffers at all. You can pass the original Buffer and Length values directly to fwprintf, no null terminators needed:

NTSTATUS __stdcall PasswordChangeNotify(
    PUNICODE_STRING UserName,
    ULONG RelativeId,
    PUNICODE_STRING NewPassword
)
{
#ifdef LOGFILE
    FILE* log = fopen(LOGFILE, "a");
    if (NULL != log)
    {
        fwprintf(log, L"%.*s password was successfully reset to %.*s\r\n",
            UserName->Length / sizeof(WCHAR), UserName->Buffer,
            NewPassword->Length / sizeof(WCHAR), NewPassword->Buffer);
        fclose(log);
    }
#endif
    return 0;
}

If you really need to add null terminators, use std::wstring instead of allocating+copying manually:

#include <string>

NTSTATUS __stdcall PasswordChangeNotify(
    PUNICODE_STRING UserName,
    ULONG RelativeId,
    PUNICODE_STRING NewPassword
)
{
#ifdef LOGFILE
    FILE* log = fopen(LOGFILE, "a");
    if (NULL != log)
    {
        fwprintf(log, L"%.s password was successfully reset to %.s\r\n",
            std::wstring(UserName->Buffer, UserName->Length / sizeof(WCHAR)).c_str(),
            std::wstring(NewPassword->Buffer, NewPassword->Length / sizeof(WCHAR)).c_str());
        fclose(log);
    }
#endif
    return 0;
}
Comments