Our application navigates through a list of named pipes and looks for a named pipe created by our application.
If our named pipe does not exist, we go ahead and create one. However, lately our clients have reported
application crashes at line:
fileName = std::wstring(TmpInfo->FileName);
You could review the windbg crash dump report that is attached below. The crash is sporadic and not reproducible at all times. Could you help me identify the issue? Is there an issue in converting TmpInfo->FileName
to wstring
. Please note that this component is an ActiveX control. Our ActiveX is crashing other applications.
The following code is responsible for enumerating named pipes.
typedef struct {
ULONG NextEntryOffset;
ULONG FileIndex;
ULONG FileNameLength;
WCHAR FileName[1];
} FILE_NAMES_INFORMATION, *PFILE_NAMES_INFORMATION;
inline void EnumerateRunningPipes(std::vector<std::wstring>& pipeNames, std::wstring stringLookup,
bool useStringLookup, bool
truncatePipeDirPrefix)
{
LONG ntStatus;
IO_STATUS_BLOCK IoStatus;
BOOL bReset = TRUE;
PFILE_NAMES_INFORMATION fileNameInfo, TmpInfo;
fileNameInfo = (PFILE_NAMES_INFORMATION) new BYTE[1024];
HANDLE hPipe = CreateFile("//./pipe/",
GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
0,
NULL);
if(hPipe == INVALID_HANDLE_VALUE)
throw CEnumeratePipeException("Could not get handle to root directory for pipes.",
CEnumeratePipeException::ErrorType::CREATE_FILE, GetLastError());
USES_CONVERSION;
locale loc;
mbstate_t state = {0};
while(true)
{
memset(fileNameInfo, 0, 1024);
ntStatus = NtQueryDirectoryFile( hPipe, NULL, NULL, NULL, &IoStatus, fileNameInfo,
1024,
FileNameInformation, FALSE, NULL, bReset );
if (ntStatus!=NO_ERROR)
{
if (ntStatus == STATUS_NO_MORE_FILES)
break;
std::stringstream sstream;
sstream << "NtQueryDirectoryFile error " << ntStatus;
logger->writeLog(sstream.str());
throw CEnumeratePipeException(
"NtQueryDirectoryFile could not get information about the directory or
file.",
CEnumeratePipeException::ErrorType::QUERY_DIR, ntStatus);
}
TmpInfo = fileNameInfo;
while(true)
{
const int endStringAt = TmpInfo->FileNameLength / sizeof(WCHAR);
std::wstring fileName;
try
{
fileName = std::wstring(TmpInfo->FileName);//The access violation
occurs at this line of code.
fileName = fileName.substr(0, endStringAt);
}
catch (...)
{
logger->writeLog("Caught unknown exception.");
if (TmpInfo->FileName == NULL)
{
logger->writeLog("TmpInfo->FileName = NULL");
}
}
if (useStringLookup)
{
if (fileName.find(stringLookup) != std::wstring::npos)
{
if (!truncatePipeDirPrefix)
fileName.insert(0, L"\\\\.\\pipe\\");
pipeNames.push_back(fileName);
}
}
else
{
pipeNames.push_back(fileName);
}
if(TmpInfo->NextEntryOffset==0)
break;
TmpInfo = (PFILE_NAMES_INFORMATION)((DWORD)TmpInfo+TmpInfo->NextEntryOffset);
}
bReset = FALSE;
}
delete fileNameInfo;
}
Here is part of the Crash Dump report:
FAULTING_IP: msvcr100!memcpy+57 [f:\dd\vctools\crt_bld\SELF_X86\crt\src\INTEL\memcpy.asm @ 185] 78aa1ed7 f3a5 rep movs dword ptr es:[edi],dword ptr [esi] EXCEPTION_RECORD: ffffffff -- (.exr 0xffffffffffffffff) ExceptionAddress: 78aa1ed7 (msvcr100!memcpy+0x00000057) ExceptionCode: c0000005 (Access violation) ExceptionFlags: 00000000 NumberParameters: 2 Parameter[0]: 00000000 Parameter[1]: 0d6b8000 Attempt to read from address 0d6b8000 SYMBOL_STACK_INDEX: 0 SYMBOL_NAME: msvcr100!memcpy+57 FOLLOWUP_NAME: MachineOwner MODULE_NAME: msvcr100 IMAGE_NAME: msvcr100.dll DEBUG_FLR_IMAGE_TIMESTAMP: 4ba1dbbe FAILURE_BUCKET_ID: STRING_DEREFERENCE_c0000005_msvcr100.dll!memcpy BUCKET_ID: APPLICATION_FAULT_STRING_DEREFERENCE_CODE_ADDRESS_MISMATCH_INVALID_POINTER_READ_WRONG_SYMBOLS_msvcr100!memcpy +57
I don't see anything in the FILE_NAMES_INFORMATION
documentation that says that the FileName
member is null-terminated. And the presence of the FileNameLength
member suggests that it is not null-terminated.
So it seems likely that
fileName = std::wstring(TmpInfo->FileName);
is reading off the end of your TmpInfo
buffer until it encounters two consecutive null bytes. If the null bytes aren't encountered before it hits an unreadable virtual memory region, you'll get an access violation.
I'd recommend replacing these two lines of code:
fileName = std::wstring(TmpInfo->FileName);
fileName = fileName.substr(0, endStringAt);
with this:
fileName = std::wstring(TmpInfo->FileName, TmpInfo->FileNameLength);
That constructor will only read FileNameLength
characters out of the buffer, so it shouldn't crash. And it's more efficient!
How are TmpInfo and TmpInfo->FileName defined? Do you have example values for TmpInfo->FileName when it crashes?
My guess is that it's not being null terminated in some cases, causing std::wstring
's constructor to read off into memory that doesn't belong to it.
User contributions licensed under CC BY-SA 3.0