Fix buffer overflows due to incorrect use of sizeof

A snippet of example code:

UTF16 pszFilter[1024];
...
convertUTF8toUTF16((UTF8 *)mData.mFilters, pszFilter, sizeof(pszFilter));

Since the conversion function is expecting the third parameter to be the
length in 16-bit characters, *not* bytes, this results in the function
writing outside the bounds of the output array.

To make this less likely to happen in the future (I hope), I've provided a
template function that infers the correct size of a static array, so it's
no longer necessary to pass the size in most cases. The sized function has
been renamed with an "N" suffix to hopefully encourage this use.

This bug was caught due to a warning from MSVC about stack corruption
occurring in codeBlock::exec(), after opening a file open dialog twice in
succession. After some hunting, I found that this was due to
FileDialog::Execute() passing incorrect buffer sizes to the conversion
function, which resulted in the function writing a null terminator into
some memory that happened to be in the stack frame of codeBlock::exec()!

Engine/source/core/stringBuffer.cpp

@@ -141,7 +141,7 @@ void StringBuffer::set(const UTF8 *in)
    incRequestCount8();
    // Convert and store. Note that a UTF16 version of the string cannot be longer.
    FrameTemp<UTF16> tmpBuff(dStrlen(in)+1);
-   if(!in || in[0] == 0 || !convertUTF8toUTF16(in, tmpBuff, dStrlen(in)+1))
+   if(!in || in[0] == 0 || !convertUTF8toUTF16N(in, tmpBuff, dStrlen(in)+1))
    {
       // Easy out, it's a blank string, or a bad string.
       mBuffer.clear();

Engine/source/core/strings/unicode.cpp

@@ -146,7 +146,7 @@ inline bool isAboveBMP(U32 codepoint)
 }
 
 //-----------------------------------------------------------------------------
-U32 convertUTF8toUTF16(const UTF8 *unistring, UTF16 *outbuffer, U32 len)
+U32 convertUTF8toUTF16N(const UTF8 *unistring, UTF16 *outbuffer, U32 len)
 {
    AssertFatal(len >= 1, "Buffer for unicode conversion must be large enough to hold at least the null terminator.");
    PROFILE_SCOPE(convertUTF8toUTF16);
@@ -252,7 +252,7 @@ UTF16* convertUTF8toUTF16( const UTF8* unistring)
    FrameTemp<UTF16> buf(len);
    
    // perform conversion
-   nCodepoints = convertUTF8toUTF16( unistring, buf, len);
+   nCodepoints = convertUTF8toUTF16N( unistring, buf, len);
    
    // add 1 for the NULL terminator the converter promises it included.
    nCodepoints++;

Engine/source/core/strings/unicode.h

@@ -79,10 +79,16 @@ UTF8*  convertUTF16toUTF8( const UTF16 *unistring);
 /// - Output is null terminated. Be sure to provide 1 extra byte, U16 or U32 for
 ///   the null terminator, or you will see truncated output.
 /// - If the provided buffer is too small, the output will be truncated.
-U32 convertUTF8toUTF16(const UTF8 *unistring, UTF16 *outbuffer, U32 len);
+U32 convertUTF8toUTF16N(const UTF8 *unistring, UTF16 *outbuffer, U32 len);
 
 U32 convertUTF16toUTF8( const UTF16 *unistring, UTF8  *outbuffer, U32 len);
 
+template <size_t N>
+inline U32 convertUTF8toUTF16(const UTF8 *unistring, UTF16 (&outbuffer)[N])
+{
+   return convertUTF8toUTF16N(unistring, outbuffer, (U32) N);
+}
+
 //-----------------------------------------------------------------------------
 /// Functions that converts one unicode codepoint at a time
 /// - Since these functions are designed to be used in tight loops, they do not

Engine/source/gfx/gFont.cpp

@@ -423,7 +423,7 @@ U32 GFont::getStrNWidth(const UTF8 *str, U32 n)
 {
    // UTF8 conversion is expensive. Avoid converting in a tight loop.
    FrameTemp<UTF16> str16(n + 1);
-   convertUTF8toUTF16(str, str16, n + 1);
+   convertUTF8toUTF16N(str, str16, n + 1);
    return getStrNWidth(str16, dStrlen(str16));
 }
 
@@ -462,7 +462,7 @@ U32 GFont::getStrNWidth(const UTF16 *str, U32 n)
 U32 GFont::getStrNWidthPrecise(const UTF8 *str, U32 n)
 {
    FrameTemp<UTF16> str16(n + 1);
-   convertUTF8toUTF16(str, str16, n + 1);
+   convertUTF8toUTF16N(str, str16, n + 1);
    return getStrNWidthPrecise(str16, dStrlen(str16));
 }
 

Engine/source/gfx/gfxDrawUtil.cpp

@@ -149,7 +149,7 @@ U32 GFXDrawUtil::drawTextN( GFont *font, const Point2I &ptDraw, const UTF8 *in_s
    // Convert to UTF16 temporarily.
    n++; // space for null terminator
    FrameTemp<UTF16> ubuf( n * sizeof(UTF16) );
-   convertUTF8toUTF16(in_string, ubuf, n);
+   convertUTF8toUTF16N(in_string, ubuf, n);
 
    return drawTextN( font, ptDraw, ubuf, n, colorTable, maxColorIndex, rot );
 }

Engine/source/platformWin32/nativeDialogs/fileDialog.cpp

@@ -324,10 +324,10 @@ bool FileDialog::Execute()
    UTF16 pszFileTitle[MAX_PATH];
    UTF16 pszDefaultExtension[MAX_PATH];
    // Convert parameters to UTF16*'s
-   convertUTF8toUTF16((UTF8 *)mData.mDefaultFile, pszFile, sizeof(pszFile));
-   convertUTF8toUTF16((UTF8 *)mData.mDefaultPath, pszInitialDir, sizeof(pszInitialDir));
-   convertUTF8toUTF16((UTF8 *)mData.mTitle, pszTitle, sizeof(pszTitle));
-   convertUTF8toUTF16((UTF8 *)mData.mFilters, pszFilter, sizeof(pszFilter) );
+   convertUTF8toUTF16((UTF8 *)mData.mDefaultFile, pszFile);
+   convertUTF8toUTF16((UTF8 *)mData.mDefaultPath, pszInitialDir);
+   convertUTF8toUTF16((UTF8 *)mData.mTitle, pszTitle);
+   convertUTF8toUTF16((UTF8 *)mData.mFilters, pszFilter);
 #else
    // Not Unicode, All char*'s!
    char pszFile[MAX_PATH];

Engine/source/platformWin32/winConsole.cpp

@@ -67,7 +67,7 @@ void WinConsole::enable(bool enabled)
       {
 #ifdef UNICODE
          UTF16 buf[512];
-         convertUTF8toUTF16((UTF8 *)title, buf, sizeof(buf));
+         convertUTF8toUTF16((UTF8 *)title, buf);
          SetConsoleTitle(buf);
 #else
          SetConsoleTitle(title);

Engine/source/platformWin32/winExec.cpp

@@ -88,15 +88,15 @@ ExecuteThread::ExecuteThread(const char *executable, const char *args /* = NULL
 
 #ifdef UNICODE
    WCHAR exe[ 1024 ];
-   convertUTF8toUTF16( exeBuf, exe, sizeof( exe ) / sizeof( exe[ 0 ] ) );
+   convertUTF8toUTF16( exeBuf, exe );
 
    TempAlloc< WCHAR > argsBuf( ( args ? dStrlen( args ) : 0 ) + 1 );
    argsBuf[ argsBuf.size - 1 ] = 0;
 
    if( args )
-      convertUTF8toUTF16( args, argsBuf, argsBuf.size );
+      convertUTF8toUTF16N( args, argsBuf, argsBuf.size );
    if( directory )
-      convertUTF8toUTF16( directory, dirBuf, dirBuf.size );
+      convertUTF8toUTF16N( directory, dirBuf, dirBuf.size );
 #else
    char* exe = exeBuf;
    char* argsBuf = args;
@@ -162,7 +162,7 @@ void Platform::openFolder(const char* path )
 
 #ifdef UNICODE
    WCHAR p[ 1024 ];
-   convertUTF8toUTF16( filePath, p, sizeof( p ) / sizeof( p[ 0 ] ) );
+   convertUTF8toUTF16( filePath, p );
 #else
    char* p = filePath;
 #endif
@@ -179,7 +179,7 @@ void Platform::openFile(const char* path )
 
 #ifdef UNICODE
    WCHAR p[ 1024 ];
-   convertUTF8toUTF16( filePath, p, sizeof( p ) / sizeof( p[ 0 ] ) );
+   convertUTF8toUTF16( filePath, p );
 #else
    char* p = filePath;
 #endif

Engine/source/platformWin32/winFileio.cpp

@@ -54,7 +54,7 @@ bool dFileDelete(const char * name)
    TempAlloc< TCHAR > buf( dStrlen( name ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( name, buf, buf.size );
+   convertUTF8toUTF16N( name, buf, buf.size );
 #else
    dStrcpy( buf, name );
 #endif
@@ -74,8 +74,8 @@ bool dFileRename(const char *oldName, const char *newName)
    TempAlloc< TCHAR > newf( dStrlen( newName ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( oldName, oldf, oldf.size );
-   convertUTF8toUTF16( newName, newf, newf.size );
+   convertUTF8toUTF16N( oldName, oldf, oldf.size );
+   convertUTF8toUTF16N( newName, newf, newf.size );
 #else
    dStrcpy(oldf, oldName);
    dStrcpy(newf, newName);
@@ -93,7 +93,7 @@ bool dFileTouch(const char * name)
    TempAlloc< TCHAR > buf( dStrlen( name ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( name, buf, buf.size );
+   convertUTF8toUTF16N( name, buf, buf.size );
 #else
    dStrcpy( buf, name );
 #endif
@@ -119,8 +119,8 @@ bool dPathCopy(const char *fromName, const char *toName, bool nooverwrite)
    TempAlloc< TCHAR > to( dStrlen( toName ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( fromName, from, from.size );
-   convertUTF8toUTF16( toName, to, to.size );
+   convertUTF8toUTF16N( fromName, from, from.size );
+   convertUTF8toUTF16N( toName, to, to.size );
 #else
    dStrcpy( from, fromName );
    dStrcpy( to, toName );
@@ -187,8 +187,8 @@ bool dPathCopy(const char *fromName, const char *toName, bool nooverwrite)
          backslash(toFile);
          
 #ifdef UNICODE
-         convertUTF8toUTF16( tempBuf, wtempBuf, wtempBuf.size );
-         convertUTF8toUTF16( tempBuf1, wtempBuf1, wtempBuf1.size );
+         convertUTF8toUTF16N( tempBuf, wtempBuf, wtempBuf.size );
+         convertUTF8toUTF16N( tempBuf1, wtempBuf1, wtempBuf1.size );
          WCHAR* f = wtempBuf1;
          WCHAR* t = wtempBuf;
 #else
@@ -256,7 +256,7 @@ File::FileStatus File::open(const char *filename, const AccessMode openMode)
    TempAlloc< TCHAR > fname( dStrlen( filename ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( filename, fname, fname.size );
+   convertUTF8toUTF16N( filename, fname, fname.size );
 #else
    dStrcpy(fname, filename);
 #endif
@@ -586,7 +586,7 @@ static bool recurseDumpPath(const char *path, const char *pattern, Vector<Platfo
 
 #ifdef UNICODE
    TempAlloc< WCHAR > searchBuf( buf.size );
-   convertUTF8toUTF16( buf, searchBuf, searchBuf.size );
+   convertUTF8toUTF16N( buf, searchBuf, searchBuf.size );
    WCHAR* search = searchBuf;
 #else
    char *search = buf;
@@ -665,7 +665,7 @@ bool Platform::getFileTimes(const char *filePath, FileTime *createTime, FileTime
    TempAlloc< TCHAR > fp( dStrlen( filePath ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( filePath, fp, fp.size );
+   convertUTF8toUTF16N( filePath, fp, fp.size );
 #else
    dStrcpy( fp, filePath );
 #endif
@@ -697,7 +697,7 @@ bool Platform::createPath(const char *file)
 
 #ifdef UNICODE
    TempAlloc< WCHAR > fileBuf( pathbuf.size );
-   convertUTF8toUTF16( file, fileBuf, fileBuf.size );
+   convertUTF8toUTF16N( file, fileBuf, fileBuf.size );
    const WCHAR* fileName = fileBuf;
    const WCHAR* dir;
 #else
@@ -820,7 +820,7 @@ bool Platform::setCurrentDirectory(StringTableEntry newDir)
    TempAlloc< TCHAR > buf( dStrlen( newDir ) + 2 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( newDir, buf, buf.size - 1 );
+   convertUTF8toUTF16N( newDir, buf, buf.size - 1 );
 #else
    dStrcpy( buf, newDir );
 #endif
@@ -935,7 +935,7 @@ bool Platform::isFile(const char *pFilePath)
    TempAlloc< TCHAR > buf( dStrlen( pFilePath ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( pFilePath, buf, buf.size );
+   convertUTF8toUTF16N( pFilePath, buf, buf.size );
 #else
    dStrcpy( buf, pFilePath );
 #endif
@@ -974,7 +974,7 @@ S32 Platform::getFileSize(const char *pFilePath)
    TempAlloc< TCHAR > buf( dStrlen( pFilePath ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( pFilePath, buf, buf.size );
+   convertUTF8toUTF16N( pFilePath, buf, buf.size );
 #else
    dStrcpy( buf, pFilePath );
 #endif
@@ -1011,7 +1011,7 @@ bool Platform::isDirectory(const char *pDirPath)
    TempAlloc< TCHAR > buf( dStrlen( pDirPath ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( pDirPath, buf, buf.size );
+   convertUTF8toUTF16N( pDirPath, buf, buf.size );
 #else
    dStrcpy( buf, pDirPath );
 #endif
@@ -1057,8 +1057,8 @@ bool Platform::isSubDirectory(const char *pParent, const char *pDir)
    TempAlloc< TCHAR > dir( dStrlen( pDir ) + 1 );
 
 #ifdef UNICODE
-   convertUTF8toUTF16( fileName, file, file.size );
-   convertUTF8toUTF16( pDir, dir, dir.size );
+   convertUTF8toUTF16N( fileName, file, file.size );
+   convertUTF8toUTF16N( pDir, dir, dir.size );
 #else
    dStrcpy( file, fileName );
    dStrcpy( dir, pDir );
@@ -1251,7 +1251,7 @@ bool Platform::hasSubDirectory(const char *pPath)
 
 #ifdef UNICODE
    WCHAR buf[ 1024 ];
-   convertUTF8toUTF16( searchBuf, buf, sizeof( buf ) / sizeof( buf[ 0 ] ) );
+   convertUTF8toUTF16( searchBuf, buf );
    WCHAR* search = buf;
 #else
    char* search = searchBuf;
@@ -1335,7 +1335,7 @@ static bool recurseDumpDirectories(const char *basePath, const char *subPath, Ve
 
 #ifdef UNICODE
    TempAlloc< WCHAR > searchStr( dStrlen( search ) + 1 );
-   convertUTF8toUTF16( search, searchStr, searchStr.size );
+   convertUTF8toUTF16N( search, searchStr, searchStr.size );
 #else
    char* searchStr = search;
 #endif

Engine/source/platformWin32/winRedbook.cpp

@@ -144,7 +144,7 @@ bool Win32RedBookDevice::open()
    openParms.lpstrDeviceType = (LPCWSTR)MCI_DEVTYPE_CD_AUDIO;
 
    UTF16 buf[512];
-   convertUTF8toUTF16((UTF8 *)mDeviceName, buf, sizeof(buf));
+   convertUTF8toUTF16((UTF8 *)mDeviceName, buf);
    openParms.lpstrElementName = buf;
 #else
    openParms.lpstrDeviceType = (LPCSTR)MCI_DEVTYPE_CD_AUDIO;

Engine/source/platformWin32/winWindow.cpp

@@ -104,7 +104,7 @@ bool Platform::excludeOtherInstances(const char *mutexName)
 {
 #ifdef UNICODE
    UTF16 b[512];
-   convertUTF8toUTF16((UTF8 *)mutexName, b, sizeof(b));
+   convertUTF8toUTF16((UTF8 *)mutexName, b);
    gMutexHandle = CreateMutex(NULL, true, b);
 #else
    gMutexHandle = CreateMutex(NULL, true, mutexName);
@@ -164,7 +164,7 @@ bool Platform::checkOtherInstances(const char *mutexName)
    
 #ifdef UNICODE
    UTF16 b[512];
-   convertUTF8toUTF16((UTF8 *)mutexName, b, sizeof(b));
+   convertUTF8toUTF16((UTF8 *)mutexName, b);
    pMutex  = CreateMutex(NULL, true, b);
 #else
    pMutex = CreateMutex(NULL, true, mutexName);
@@ -197,8 +197,8 @@ void Platform::AlertOK(const char *windowTitle, const char *message)
    ShowCursor(true);
 #ifdef UNICODE
    UTF16 m[1024], t[512];
-   convertUTF8toUTF16((UTF8 *)windowTitle, t, sizeof(t));
-   convertUTF8toUTF16((UTF8 *)message, m, sizeof(m));
+   convertUTF8toUTF16((UTF8 *)windowTitle, t);
+   convertUTF8toUTF16((UTF8 *)message, m);
    MessageBox(NULL, m, t, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_OK);
 #else
    MessageBox(NULL, message, windowTitle, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_OK);
@@ -211,8 +211,8 @@ bool Platform::AlertOKCancel(const char *windowTitle, const char *message)
    ShowCursor(true);
 #ifdef UNICODE
    UTF16 m[1024], t[512];
-   convertUTF8toUTF16((UTF8 *)windowTitle, t, sizeof(t));
-   convertUTF8toUTF16((UTF8 *)message, m, sizeof(m));
+   convertUTF8toUTF16((UTF8 *)windowTitle, t);
+   convertUTF8toUTF16((UTF8 *)message, m);
    return MessageBox(NULL, m, t, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_OKCANCEL) == IDOK;
 #else
    return MessageBox(NULL, message, windowTitle, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_OKCANCEL) == IDOK;
@@ -225,8 +225,8 @@ bool Platform::AlertRetry(const char *windowTitle, const char *message)
    ShowCursor(true);
 #ifdef UNICODE
    UTF16 m[1024], t[512];
-   convertUTF8toUTF16((UTF8 *)windowTitle, t, sizeof(t));
-   convertUTF8toUTF16((UTF8 *)message, m, sizeof(m));
+   convertUTF8toUTF16((UTF8 *)windowTitle, t);
+   convertUTF8toUTF16((UTF8 *)message, m);
    return (MessageBox(NULL, m, t, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_RETRYCANCEL) == IDRETRY);
 #else
    return (MessageBox(NULL, message, windowTitle, MB_ICONINFORMATION | MB_SETFOREGROUND | MB_TASKMODAL | MB_RETRYCANCEL) == IDRETRY);
@@ -241,8 +241,8 @@ Platform::ALERT_ASSERT_RESULT Platform::AlertAssert(const char *windowTitle, con
 
 #ifdef UNICODE
    UTF16 messageUTF[1024], title[512];
-   convertUTF8toUTF16((UTF8 *)windowTitle, title, sizeof(title));
-   convertUTF8toUTF16((UTF8 *)message, messageUTF, sizeof(messageUTF));
+   convertUTF8toUTF16((UTF8 *)windowTitle, title);
+   convertUTF8toUTF16((UTF8 *)message, messageUTF);
 #else
    const char* messageUTF = message;
    const char* title = windowTitle;
@@ -560,7 +560,7 @@ bool Platform::openWebBrowser( const char* webAddress )
 #ifdef UNICODE
    dSprintf( buf, sizeof( buf ), "%s %s", utf8WebKey, webAddress );   
    UTF16 b[1024];
-   convertUTF8toUTF16((UTF8 *)buf, b, sizeof(b));
+   convertUTF8toUTF16((UTF8 *)buf, b);
 #else
    dSprintf( buf, sizeof( buf ), "%s %s", sWebKey, webAddress );   
 #endif