From 19682f1b58120534a28d2d6ed8e23e37927ffb08 Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Tue, 5 Jan 2016 11:20:35 -0800 Subject: [PATCH] Fix buffer overrun and _atoi64 in ildasm PrettyPrintSigature allocates only 16 byte char array for prefix name, but the input string was synthesized from an 64 bit address -- szVarPrefix or szArgPrefix. The maximum number of decimal digits for 64 bit value is 21, which overflows the allocated buffer. Note actually ildasm even prepends '@' and appends '0' for the prefix name. The fix is to declare MAX_PREFIX_SIZE as 32 and use it everywhere for the purpose. This also fixes '_atoi64' which actually returns 32 bit value using '{PAL_}atol' in Unix. Instead, I imports 'atoll' for the use of '_atoi64', which correctly converts string to 64 bit integer. --- src/ildasm/dasm.cpp | 10 +++------- src/ildasm/dis.cpp | 10 +++------- src/inc/formattype.cpp | 4 ++-- src/inc/formattype.h | 2 ++ src/pal/inc/pal.h | 1 + src/pal/inc/pal_mstypes.h | 3 +-- 6 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/ildasm/dasm.cpp b/src/ildasm/dasm.cpp index 8032af63d5..a220fa86bf 100644 --- a/src/ildasm/dasm.cpp +++ b/src/ildasm/dasm.cpp @@ -3447,7 +3447,7 @@ BOOL DumpMethod(mdToken FuncToken, const char *pszClassName, DWORD dwEntryPointT ULONG ulArgs=0; unsigned retParamIx = 0; unsigned uStringLen = SZSTRING_SIZE; - char szArgPrefix[32]; + char szArgPrefix[MAX_PREFIX_SIZE]; char* szptr = NULL; mdToken tkMVarOwner = g_tkMVarOwner; @@ -3627,7 +3627,7 @@ lDone: ; qbMemberSig.Shrink(0); // Get the argument names, if any - strcpy_s(szArgPrefix,32,(g_fThisIsInstanceMethod ? "A1": "A0")); + strcpy_s(szArgPrefix,MAX_PREFIX_SIZE,(g_fThisIsInstanceMethod ? "A1": "A0")); { PCCOR_SIGNATURE typePtr = pComSig; unsigned ulCallConv = CorSigUncompressData(typePtr); // get the calling convention out of the way @@ -3699,11 +3699,7 @@ lDone: ; sprintf_s(pszArgname[j].name,16,"A_%d",g_fThisIsInstanceMethod ? j+1 : j); } }// end for( along the argnames) -#ifdef _WIN64 - sprintf_s(szArgPrefix,32,"@%I64d0",(size_t)pszArgname); -#else - sprintf_s(szArgPrefix,32,"@%d0",(size_t)pszArgname); -#endif //_WIN64 + sprintf_s(szArgPrefix,MAX_PREFIX_SIZE,"@%Id0",(size_t)pszArgname); } //end if (ulArgs) g_pImport->EnumClose(&hArgEnum); } diff --git a/src/ildasm/dis.cpp b/src/ildasm/dis.cpp index 796271b356..48f4586ab9 100644 --- a/src/ildasm/dis.cpp +++ b/src/ildasm/dis.cpp @@ -915,7 +915,7 @@ BOOL Disassemble(IMDInternalImport *pImport, BYTE *ILHeader, void *GUICookie, md LineCodeDescr* pLCD = NULL; ParamDescriptor* pszLVname = NULL; ULONG ulVars=0; - char szVarPrefix[64]; + char szVarPrefix[MAX_PREFIX_SIZE]; // scope handling: DynamicArray daScope; ULONG ulScopes=0; @@ -928,7 +928,7 @@ BOOL Disassemble(IMDInternalImport *pImport, BYTE *ILHeader, void *GUICookie, md ULONG32 ulMethodCol[2]; BOOL fHasRangeInfo = FALSE; - strcpy_s(szVarPrefix,64,"V0"); + strcpy_s(szVarPrefix,MAX_PREFIX_SIZE,"V0"); if(g_pSymReader) { g_pSymReader->GetMethod(FuncToken,&pSymMethod); @@ -1048,11 +1048,7 @@ BOOL Disassemble(IMDInternalImport *pImport, BYTE *ILHeader, void *GUICookie, md LoadScope(pRootScope,&daScope,&ulScopes); qsort(&daScope[0],ulScopes,sizeof(LexScope),cmpLexScope); OpenScope(pRootScope,pszLVname,ulVars); -#ifdef _WIN64 - sprintf_s(szVarPrefix,64,"@%I64d0",(size_t)pszLVname); -#else - sprintf_s(szVarPrefix,64,"@%d0",(size_t)pszLVname); -#endif //_WIN64 + sprintf_s(szVarPrefix,MAX_PREFIX_SIZE,"@%Id0",(size_t)pszLVname); #ifndef SHOW_LEXICAL_SCOPES for(unsigned jjj = 0; jjj < ulScopes; jjj++) diff --git a/src/inc/formattype.cpp b/src/inc/formattype.cpp index 923ff3e9a1..b8960f48c6 100644 --- a/src/inc/formattype.cpp +++ b/src/inc/formattype.cpp @@ -206,14 +206,14 @@ const PCCOR_SIGNATURE PrettyPrintSignature( PCCOR_SIGNATURE typeEnd = typePtr + typeLen; unsigned ixArg= 0; //arg index char argname[1024]; - char label[16]; + char label[MAX_PREFIX_SIZE]; const char* openpar = "("; const char* closepar = ")"; ParamDescriptor* pszArgName = NULL; // ptr to array of names (if provided by debug info) if(inlabel && *inlabel) // check for *inlabel is totally unnecessary, added to pacify the PREFIX { - strcpy_s(label,COUNTOF(label),inlabel); + strcpy_s(label,MAX_PREFIX_SIZE,inlabel); ixArg = label[strlen(label)-1] - '0'; label[strlen(label)-1] = 0; if(label[0] == '@') // it's pointer! diff --git a/src/inc/formattype.h b/src/inc/formattype.h index 739bc89dd4..53ba4a115c 100644 --- a/src/inc/formattype.h +++ b/src/inc/formattype.h @@ -16,6 +16,8 @@ #endif #endif +#define MAX_PREFIX_SIZE 32 + struct ParamDescriptor { char* name; diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h index c139efcba8..7eaba46688 100644 --- a/src/pal/inc/pal.h +++ b/src/pal/inc/pal.h @@ -6112,6 +6112,7 @@ PALIMPORT int __cdecl vsprintf(char *, const char *, va_list); PALIMPORT int __cdecl sscanf(const char *, const char *, ...); PALIMPORT int __cdecl atoi(const char *); PALIMPORT LONG __cdecl atol(const char *); +PALIMPORT long long int __cdecl atoll(const char *); PALIMPORT ULONG __cdecl strtoul(const char *, char **, int); PALIMPORT double __cdecl atof(const char *); PALIMPORT double __cdecl strtod(const char *, char **); diff --git a/src/pal/inc/pal_mstypes.h b/src/pal/inc/pal_mstypes.h index aca1c60814..e56cf1bd6d 100644 --- a/src/pal/inc/pal_mstypes.h +++ b/src/pal/inc/pal_mstypes.h @@ -319,8 +319,7 @@ typedef signed __int64 LONG64; #ifdef BIT64 -// UNIXTODO: Implement proper _atoi64, the atol returns 32 bit result -#define _atoi64 (__int64)atol +#define _atoi64 (__int64)atoll typedef __int64 INT_PTR, *PINT_PTR; typedef unsigned __int64 UINT_PTR, *PUINT_PTR; -- 2.34.1