diff --git a/Ghidra/Features/PDB/src/pdb/cpp/iterate.cpp b/Ghidra/Features/PDB/src/pdb/cpp/iterate.cpp index b9af9064f1..9ade77de1c 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/iterate.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/iterate.cpp @@ -64,7 +64,7 @@ void iterateEnums() { BSTR type = getTypeAsString(pSymbol); ULONGLONG len = getLength(pSymbol); - printf("%s\n", indent(8), name, type, len); + printf("%s\n", indent(8), name, type, len); iterateEnumMembers(pSymbol); @@ -89,7 +89,7 @@ void iterateMembers(IDiaSymbol * pSymbol) { if (celt != 1) { break; } - printf("%s\n", + printf("%s\n", indent(12), getName(pMember), getTypeAsString(pMember), @@ -133,7 +133,7 @@ void iterateDataTypes() { continue; } - printf("%s\n", indent(8), name, kind, len); + printf("%s\n", indent(8), name, kind, len); iterateMembers(pSymbol); @@ -207,7 +207,7 @@ void iterateClasses() { continue; } - printf("%s\n", indent(8), name, len); + printf("%s\n", indent(8), name, len); iterateMembers(pSymbol); @@ -235,7 +235,7 @@ void dumpFunctionStackVariables( DWORD rva ) while ( pEnum != NULL && SUCCEEDED( pEnum->Next( 1, &pSymbol, &celt ) ) && celt == 1 ) { pSymbol->get_symTag( &tag ); if ( tag == SymTagData ) { - printf("%s\n", + printf("%s\n", indent(12), getName(pSymbol), getKindAsString(pSymbol), @@ -355,7 +355,7 @@ void iterateFunctions() { DWORD address = getRVA(pSymbol); ULONGLONG len = getLength(pSymbol); - printf("%s\n", indent(8), name, address, len); + printf("%s\n", indent(8), name, address, len); dumpFunctionStackVariables(address); dumpFunctionLines(pSymbol, pSession); @@ -387,7 +387,7 @@ void iterateSymbolTable(IDiaEnumSymbols * pSymbols) { printf("%s", indent(12)); printf("get_addressSection( &isect ); pSecContrib->get_addressOffset( &offset ); pSecContrib = NULL; - IDiaSymbol * pSym; if ( pSession->findSymbolByAddr( isect, offset, SymTagNull, &pSym ) != S_OK ) { pSym = NULL; } @@ -512,7 +511,7 @@ void iterateInjectedSource(IDiaEnumInjectedSources * pInjectedSrcs) { ULONGLONG length; pInjectedSrc->get_length(&length); - printf("%s\n", + printf("%s\n", indent(8), filename, objectname, diff --git a/Ghidra/Features/PDB/src/pdb/cpp/pdb.cpp b/Ghidra/Features/PDB/src/pdb/cpp/pdb.cpp index b08daa7b14..cebd278916 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/pdb.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/pdb.cpp @@ -17,6 +17,7 @@ #include "find.h" #include "print.h" #include "symbol.h" +#include IDiaSession * pSession;//Provides a query context for debug symbols IDiaSymbol * pGlobal; @@ -51,14 +52,16 @@ int init(const char * szFilename, const char * szSignature, const char * szAge) break; default: char msg[256]; - sprintf(msg, "Unspecified error occurred: 0x%x\n", hr); + sprintf_s(msg, 256, "Unspecified error occurred: 0x%lx\n", hr); fatal(msg); break; } } - wchar_t wszFilename[ _MAX_PATH ]; - mbstowcs( wszFilename, szFilename, sizeof( wszFilename ) ); + size_t sz = strlen(szFilename) + 1; + wchar_t* wszFilename = new wchar_t[sz]; + size_t outsz; + mbstowcs_s(&outsz, wszFilename, sz, szFilename, sz - 1); if (szSignature == NULL && szAge == NULL) { hr = pSource->loadDataFromPdb( wszFilename ); @@ -81,6 +84,7 @@ int init(const char * szFilename, const char * szSignature, const char * szAge) fatal("Invalid combination of GUID/Signature/Age parameters specified!"); } checkErr(hr); + delete [] wszFilename; if (pSource->openSession( &pSession ) < 0) { fatal("Unable to open session\n"); @@ -100,13 +104,19 @@ int init(const char * szFilename, const char * szSignature, const char * szAge) GUID currGUID; BSTR guidString; - DWORD currAge; + DWORD currAge = 0; // Include PDB GUID and age in XML output for compatibility checking if (pGlobal->get_guid( &currGUID ) == S_OK) { - size_t maxGUIDStrLen = 64; + int maxGUIDStrLen = 64; wchar_t * guidStr = (wchar_t *)calloc(maxGUIDStrLen, sizeof(wchar_t)); - StringFromGUID2(currGUID, guidStr, maxGUIDStrLen); + if (guidStr == NULL) { + fatal("Unable to allocate GUID\n"); + exit(-1); + } + if (StringFromGUID2(currGUID, guidStr, maxGUIDStrLen) <= 0) { + fatal("Unable to convert GUID\n"); + } guidString = guidStr; if (pGlobal->get_age( &currAge ) == S_OK) { diff --git a/Ghidra/Features/PDB/src/pdb/cpp/print.cpp b/Ghidra/Features/PDB/src/pdb/cpp/print.cpp index e443643a7a..cf3af867d0 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/print.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/print.cpp @@ -34,68 +34,71 @@ wchar_t * printVariant( VARIANT & v ) { return L"null"; } - size_t printfFormatBufferLen = 100; - wchar_t * variant = (wchar_t *)calloc(printfFormatBufferLen, sizeof(wchar_t)); + size_t blen = 100; + wchar_t * variant = (wchar_t *)calloc(blen, sizeof(wchar_t)); + if (variant == NULL) { + return L"error"; + } switch( v.vt ) { case VT_ARRAY://Indicates a SAFEARRAY pointer. - swprintf(variant, L"%d", v.parray);//TODO + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.parray);//TODO return variant; case VT_BYREF://Indicates that a value is a reference. - swprintf(variant, L"%d", v.cVal);//TODO + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.cVal);//TODO return variant; case VT_CY://Indicates a currency value. - swprintf(variant, L"%f", v.cyVal); + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.cyVal.int64); return variant; case VT_DATE://Indicates a DATE value. - swprintf(variant, L"%d", v.date); + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.date); return variant; case VT_DISPATCH://Indicates an IDispatch pointer. - swprintf(variant, L"%d", v.pdispVal); + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.pdispVal); return variant; case VT_ERROR://Indicates an SCODE. - swprintf(variant, L"0x%x", v.scode); + swprintf_s(variant, blen, L"0x%x", v.scode); return variant; case VT_I1://CHAR - swprintf(variant, L"%d", v.cVal); + swprintf_s(variant, blen, L"%d", v.cVal); return variant; case VT_I2://SHORT - swprintf(variant, L"%d", v.iVal); + swprintf_s(variant, blen, L"%d", v.iVal); return variant; case VT_I4://LONG - swprintf(variant, L"%d", v.lVal ); + swprintf_s(variant, blen, L"%d", v.lVal ); return variant; case VT_I8: //LONGLONG - swprintf(variant, L"%ld", v.llVal ); + swprintf_s(variant, blen, L"%I64d", v.llVal ); return variant; case VT_INT://INT - swprintf(variant, L"%d", v.intVal); + swprintf_s(variant, blen, L"%d", v.intVal); return variant; case VT_R4://Indicates a float value. - swprintf(variant, L"%f", v.fltVal); + swprintf_s(variant, blen, L"%f", v.fltVal); return variant; case VT_R8://Indicates a double value. - swprintf(variant, L"%f", v.dblVal); + swprintf_s(variant, blen, L"%f", v.dblVal); return variant; case VT_UI1://BYTE - swprintf(variant, L"%d", v.bVal); + swprintf_s(variant, blen, L"%d", v.bVal); return variant; case VT_UI2://USHORT - swprintf(variant, L"%d", v.uiVal); + swprintf_s(variant, blen, L"%d", v.uiVal); return variant; case VT_UI4://ULONG - swprintf(variant, L"%d", v.ulVal); + swprintf_s(variant, blen, L"%d", v.ulVal); return variant; case VT_UI8://ULONGLONG - swprintf(variant, L"%ld", v.ullVal); + swprintf(variant, blen, L"%I64d", v.ullVal); return variant; case VT_UINT://UINT - swprintf(variant, L"%d", v.uintVal); + swprintf_s(variant, blen, L"%d", v.uintVal); return variant; case VT_UNKNOWN://Indicates an IUnknown pointer. - swprintf(variant, L"%d", v.punkVal); + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.punkVal); return variant; case VT_VARIANT://Indicates a VARIANT far pointer. - swprintf(variant, L"%d", v.pvarVal); + swprintf_s(variant, blen, L"%I64d", (ULONGLONG) v.pvarVal); return variant; default: @@ -134,8 +137,10 @@ BSTR printType( IDiaSymbol * pType, BSTR suffix ) { if ( pType->get_type( &pBaseType ) == S_OK ) { size_t length = wcslen(suffix) + 3; // length of: suffix + " *\0" wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"%ws *", suffix ); - return (BSTR)printType( pBaseType, (BSTR)str ); + if (str != NULL) { + swprintf_s(str, length, L"%ws *", suffix); + return (BSTR)printType(pBaseType, (BSTR)str); + } } else { return L""; @@ -146,8 +151,10 @@ BSTR printType( IDiaSymbol * pType, BSTR suffix ) { BSTR bt = getBaseTypeAsString( pType ); size_t length = wcslen(bt) + wcslen(suffix) + 1; // length of: bt + suffix + "\0" wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"%ws%ws", bt, suffix ); - return str; + if (str != NULL) { + swprintf_s(str, length, L"%ws%ws", bt, suffix); + return str; + } } if ( tag == SymTagArrayType ) { @@ -162,9 +169,10 @@ BSTR printType( IDiaSymbol * pType, BSTR suffix ) { } size_t strLen = wcslen(suffix) + 64 + 3; // length of suffix + wag_for_numeric_value + "[]\0" wchar_t * str = (wchar_t *)calloc(strLen, sizeof(wchar_t)); - swprintf(str, L"%ws[%ld]", suffix, lenArray/lenElem ); - - return printType( pBaseType, (BSTR)str); + if (str != NULL) { + swprintf_s(str, strLen, L"%ws[%I64d]", suffix, lenArray / lenElem); + return printType(pBaseType, (BSTR)str); + } } if ( tag == SymTagFunctionType ) { @@ -175,17 +183,22 @@ BSTR printType( IDiaSymbol * pType, BSTR suffix ) { DWORD id; DWORD rec; GUID guid; - if ( pType->get_guid( &guid ) == S_OK ) { - size_t maxGUIDStrLen = 64; - wchar_t * guidStr = (wchar_t *)calloc(maxGUIDStrLen, sizeof(wchar_t)); - StringFromGUID2(guid, guidStr, maxGUIDStrLen); - return (BSTR)guidStr; + if (pType->get_guid(&guid) == S_OK) { + int maxGUIDStrLen = 64; + wchar_t* guidStr = (wchar_t*)calloc((size_t)maxGUIDStrLen, sizeof(wchar_t)); + if (guidStr != NULL) { + if (StringFromGUID2(guid, guidStr, maxGUIDStrLen) > 0) { + return (BSTR)guidStr; + } + } } else if ( pType->get_oemId( &id ) == S_OK && pType->get_oemSymbolId( &rec ) == S_OK ) { size_t strLen = 256; // wag_for_2_hex_numbers "0xNNNNN:0xNNNNN" wchar_t * str = (wchar_t *)calloc(strLen, sizeof(wchar_t)); - swprintf(str, L"0x%x:0x%x", id, rec); - return (BSTR)str; + if (str != NULL) { + swprintf_s(str, strLen, L"0x%x:0x%x", id, rec); + return (BSTR)str; + } } return L""; } @@ -193,8 +206,10 @@ BSTR printType( IDiaSymbol * pType, BSTR suffix ) { if ( name != NULL ) { size_t length = wcslen(name) + wcslen(suffix) + 1; // length of name + suffix + "\0" wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"%ws%ws", name, suffix); - return (BSTR)str; + if (str != NULL) { + swprintf_s(str, length, L"%ws%ws", name, suffix); + return (BSTR)str; + } } return L"Undefined"; diff --git a/Ghidra/Features/PDB/src/pdb/cpp/symbol.cpp b/Ghidra/Features/PDB/src/pdb/cpp/symbol.cpp index 8e86efc285..98d082540f 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/symbol.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/symbol.cpp @@ -168,15 +168,17 @@ static wchar_t* BASIC_TYPE_STRINGS [] = { }; BSTR getName(IDiaSymbol * pSymbol) { - BSTR name, escapedName; + BSTR name, escapedName = NULL; DWORD symIndexId, locType; ULONGLONG len; if (pSymbol->get_name( &name ) == 0) { if (wcscmp(name, L"") == 0) { size_t length = 7; // length of: "NONAME\0" wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"NONAME"); - escapedName = escapeXmlEntities(str); + if (str != NULL) { + swprintf_s(str, length, L"NONAME"); + escapedName = escapeXmlEntities(str); + } } else if(wcsstr(name, L"unnamed-tag") != NULL){ if(pSymbol->get_symIndexId(&symIndexId) != 0){ @@ -184,16 +186,20 @@ BSTR getName(IDiaSymbol * pSymbol) { } size_t length = 16; // length of: "\0" + 1 extra wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"", symIndexId); - escapedName = escapeXmlEntities(str); + if (str != NULL) { + swprintf_s(str, length, L"", symIndexId); + escapedName = escapeXmlEntities(str); + } } else if(pSymbol->get_locationType(&locType) == 0 && locType == LocIsBitField && pSymbol->get_length(&len) == 0){ size_t length = wcslen(name) + 4 + 32; // length of: name + ":0x\0" + wag_hex_numeric_str_len wchar_t * str = (wchar_t *)calloc(length, sizeof(wchar_t)); - swprintf(str, L"%ws:0x%x", name, len); - escapedName = escapeXmlEntities(str); + if (str != NULL) { + swprintf_s(str, length, L"%ws:0x%I64x", name, len); + escapedName = escapeXmlEntities(str); + } } else { escapedName = escapeXmlEntities(name); } diff --git a/Ghidra/Features/PDB/src/pdb/cpp/util.cpp b/Ghidra/Features/PDB/src/pdb/cpp/util.cpp index 2f5f58153d..d899aefff9 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/util.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/util.cpp @@ -15,7 +15,7 @@ */ #include "util.h" -void zero_out(const char * str, int len) { +void zero_out(const char * str, size_t len) { char * tmp = (char *)str; for (int i = 0 ; i < len ; ++i) { tmp[i] = '\0'; @@ -39,14 +39,18 @@ int find_char(const char * str, char c) { */ int atog(GUID * guid, const char * szGUID) { char * tmp = (char *)szGUID; - char buff[256]; + size_t sz = 256; + char * buff = (char *) calloc(sz, sizeof(char)); + if (buff == NULL) { + return -1; + } /*****************************************/ int index = find_char(tmp, '-'); if (index == -1) { return -1; } - zero_out(buff, 256); - strncpy(buff, tmp, index); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, index); //the value could be too large and cause overflow. eg, "9b8c55da" if (strlen(buff) == 8 && buff[0] >= '8') { @@ -72,8 +76,8 @@ int atog(GUID * guid, const char * szGUID) { if (index == -1) { return -1; } - zero_out(buff, 256); - strncpy(buff, tmp, index); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, index); guid->Data2 = (unsigned short)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+index+1; @@ -81,50 +85,52 @@ int atog(GUID * guid, const char * szGUID) { if (index == -1) { return -1; } - zero_out(buff, 256); - strncpy(buff, tmp, index); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, index); guid->Data3 = (unsigned short)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+index+1; index = find_char(tmp, '-'); - if (index == -1) { + if (index == -1 || index >= sz) { + free(buff); return -1; } - zero_out(buff, 256); - strncpy(buff, tmp, index); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, index); int ivalue = strtol(buff, NULL, 16); guid->Data4[0] = ivalue >> 8; guid->Data4[1] = ivalue & 0xff; /*****************************************/ tmp = tmp+index+1; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[2] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+2; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[3] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+2; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[4] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+2; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[5] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+2; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[6] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ tmp = tmp+2; - zero_out(buff, 256); - strncpy(buff, tmp, 2); + zero_out(buff, sz); + strncpy_s(buff, sz, tmp, 2); guid->Data4[7] = (unsigned char)strtol(buff, NULL, 16); /*****************************************/ + free(buff); return 0; } diff --git a/Ghidra/Features/PDB/src/pdb/cpp/xml.cpp b/Ghidra/Features/PDB/src/pdb/cpp/xml.cpp index 26c667cdba..9a31a7aadf 100644 --- a/Ghidra/Features/PDB/src/pdb/cpp/xml.cpp +++ b/Ghidra/Features/PDB/src/pdb/cpp/xml.cpp @@ -15,33 +15,35 @@ */ #include "xml.h" -char * indent(int nSpaces) { +char* indent(size_t nSpaces) { switch (nSpaces) { - case 1: return " "; - case 2: return " "; - case 4: return " "; - case 6: return " "; - case 8: return " "; - case 10: return " "; - case 12: return " "; + case 1: return " "; + case 2: return " "; + case 4: return " "; + case 6: return " "; + case 8: return " "; + case 10: return " "; + case 12: return " "; } - + // NOTE: memory leak if following code is hit, but luckily there are no callers // that use indent() that would trigger this. // Probably would be better to throw an error if a non-standard nSpaces value is used. if (nSpaces < 0) { nSpaces = 0; } - char * indent = (char *)calloc(nSpaces+1, sizeof(char)); - for (int i = 0 ; i < nSpaces ; ++i) { - indent[i] = ' '; + char* indent = (char*)calloc(nSpaces + 1, sizeof(char)); + if (indent != NULL) { + for (int i = 0; i < nSpaces; ++i) { + indent[i] = ' '; + } } return indent; } BSTR escapeXmlEntities(BSTR bstr) { WCHAR * str = (WCHAR *)bstr; - int len = wcslen(str); + size_t len = wcslen(str); if (len == 0) return str; size_t destLen = 0; @@ -69,30 +71,34 @@ BSTR escapeXmlEntities(BSTR bstr) { break; } } - - WCHAR * newstr = (WCHAR *)calloc(destLen + 1, sizeof(WCHAR)); + destLen += 1; + + WCHAR * newstr = (WCHAR *)calloc(destLen, sizeof(WCHAR)); + if (newstr == NULL) { + return newstr; + } WCHAR * tmp = newstr; for (int i = 0 ; i < len ; ++i) { switch (str[i]) { case '&' : - wcscpy(tmp, L"&"); + wcscpy_s(tmp, destLen, L"&"); tmp += 5; break; case '<' : - wcscpy(tmp, L"<"); + wcscpy_s(tmp, destLen, L"<"); tmp += 4; break; case '>' : - wcscpy(tmp, L">"); + wcscpy_s(tmp, destLen, L">"); tmp += 4; break; case '\'' : - wcscpy(tmp, L"'"); + wcscpy_s(tmp, destLen, L"'"); tmp += 6; break; case '"' : - wcscpy(tmp, L"""); + wcscpy_s(tmp, destLen, L"""); tmp += 6; break; case 0x7F : diff --git a/Ghidra/Features/PDB/src/pdb/headers/xml.h b/Ghidra/Features/PDB/src/pdb/headers/xml.h index 78a644010a..8ef7957e4f 100644 --- a/Ghidra/Features/PDB/src/pdb/headers/xml.h +++ b/Ghidra/Features/PDB/src/pdb/headers/xml.h @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,9 +17,9 @@ #define __PDB__XML__H__ #include -#include "comutil.h" +#include -char * indent(int nSpaces); +char * indent(size_t nSpaces); BSTR escapeXmlEntities(BSTR bstr); #endif