From 043844719463519544a96c074df2a763b4668b4a Mon Sep 17 00:00:00 2001 From: Don HO Date: Fri, 8 Feb 2019 22:13:12 +0100 Subject: [PATCH] [EU-FOSSA] Fix stack buffer overflow on LB_GETTEXT --- PowerEditor/src/MISC/RegExt/regExtDlg.cpp | 15 ++++++- .../WinControls/ColourPicker/WordStyleDlg.cpp | 20 ++++++++-- PowerEditor/src/WinControls/Grid/BabyGrid.cpp | 39 ++++++++++++++++--- .../WinControls/Preference/preferenceDlg.cpp | 18 +++++++-- .../WinControls/Preference/preferenceDlg.h | 2 +- 5 files changed, 80 insertions(+), 14 deletions(-) diff --git a/PowerEditor/src/MISC/RegExt/regExtDlg.cpp b/PowerEditor/src/MISC/RegExt/regExtDlg.cpp index 411b6bd6..8c2b3dbc 100644 --- a/PowerEditor/src/MISC/RegExt/regExtDlg.cpp +++ b/PowerEditor/src/MISC/RegExt/regExtDlg.cpp @@ -149,6 +149,10 @@ INT_PTR CALLBACK RegExtDlg::run_dlgProc(UINT Message, WPARAM wParam, LPARAM lPar if (!_isCustomize) { auto index2Add = ::SendDlgItemMessage(_hSelf, IDC_REGEXT_LANGEXT_LIST, LB_GETCURSEL, 0, 0); + auto lbTextLen = ::SendDlgItemMessage(_hSelf, IDC_REGEXT_LANGEXT_LIST, LB_GETTEXTLEN, index2Add, 0); + if (lbTextLen > extNameMax - 1) + return TRUE; + ::SendDlgItemMessage(_hSelf, IDC_REGEXT_LANGEXT_LIST, LB_GETTEXT, index2Add, reinterpret_cast(ext2Add)); addExt(ext2Add); ::SendDlgItemMessage(_hSelf, IDC_REGEXT_LANGEXT_LIST, LB_DELETESTRING, index2Add, 0); @@ -171,6 +175,10 @@ INT_PTR CALLBACK RegExtDlg::run_dlgProc(UINT Message, WPARAM wParam, LPARAM lPar { TCHAR ext2Sup[extNameMax] = TEXT(""); auto index2Sup = ::SendDlgItemMessage(_hSelf, IDC_REGEXT_REGISTEREDEXTS_LIST, LB_GETCURSEL, 0, 0); + auto lbTextLen = ::SendDlgItemMessage(_hSelf, IDC_REGEXT_REGISTEREDEXTS_LIST, LB_GETTEXTLEN, index2Sup, 0); + if (lbTextLen > extNameMax - 1) + return TRUE; + ::SendDlgItemMessage(_hSelf, IDC_REGEXT_REGISTEREDEXTS_LIST, LB_GETTEXT, index2Sup, reinterpret_cast(ext2Sup)); if (deleteExts(ext2Sup)) ::SendDlgItemMessage(_hSelf, IDC_REGEXT_REGISTEREDEXTS_LIST, LB_DELETESTRING, index2Sup, 0); @@ -222,7 +230,12 @@ INT_PTR CALLBACK RegExtDlg::run_dlgProc(UINT Message, WPARAM wParam, LPARAM lPar { if (i != LB_ERR) { - TCHAR itemName[32]; + const size_t itemNameLen = 32; + TCHAR itemName[itemNameLen + 1]; + auto lbTextLen = ::SendDlgItemMessage(_hSelf, LOWORD(wParam), LB_GETTEXTLEN, i, 0); + if (lbTextLen > itemNameLen) + return TRUE; + ::SendDlgItemMessage(_hSelf, LOWORD(wParam), LB_GETTEXT, i, reinterpret_cast(itemName)); if (!generic_stricmp(defExtArray[nbSupportedLang-1][0], itemName)) diff --git a/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp b/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp index 8b011cda..5555fef3 100644 --- a/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp +++ b/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp @@ -494,7 +494,12 @@ int WordStyleDlg::whichTabColourIndex() auto i = ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETCURSEL, 0, 0); if (i == LB_ERR) return -1; - TCHAR styleName[128]; + const size_t styleNameLen = 128; + TCHAR styleName[styleNameLen + 1]; + auto lbTextLen = ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETTEXTLEN, i, 0); + if (lbTextLen > styleNameLen) + return -1; + ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETTEXT, i, reinterpret_cast(styleName)); if (lstrcmp(styleName, TABBAR_ACTIVEFOCUSEDINDCATOR) == 0) @@ -709,19 +714,28 @@ void WordStyleDlg::setVisualFromStyleList() //bool showWarning = ((_currentLexerIndex == 0) && (style._styleID == STYLE_DEFAULT));//?SW_SHOW:SW_HIDE; COLORREF c = RGB(0x00, 0x00, 0xFF); - TCHAR str[256]; + const size_t strLen = 256; + TCHAR str[strLen + 1]; str[0] = '\0'; auto i = ::SendDlgItemMessage(_hSelf, IDC_LANGUAGES_LIST, LB_GETCURSEL, 0, 0); if (i == LB_ERR) return; + auto lbTextLen = ::SendDlgItemMessage(_hSelf, IDC_LANGUAGES_LIST, LB_GETTEXTLEN, i, 0); + if (lbTextLen > strLen) + return; + ::SendDlgItemMessage(_hSelf, IDC_LANGUAGES_LIST, LB_GETTEXT, i, reinterpret_cast(str)); i = ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETCURSEL, 0, 0); if (i == LB_ERR) return; - TCHAR styleName[64]; + const size_t styleNameLen = 64; + TCHAR styleName[styleNameLen + 1]; + lbTextLen = ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETTEXTLEN, i, 0); + if (lbTextLen > styleNameLen) + return; ::SendDlgItemMessage(_hSelf, IDC_STYLES_LIST, LB_GETTEXT, i, reinterpret_cast(styleName)); lstrcat(lstrcat(str, TEXT(" : ")), styleName); diff --git a/PowerEditor/src/WinControls/Grid/BabyGrid.cpp b/PowerEditor/src/WinControls/Grid/BabyGrid.cpp index f5791a19..c68370f4 100644 --- a/PowerEditor/src/WinControls/Grid/BabyGrid.cpp +++ b/PowerEditor/src/WinControls/Grid/BabyGrid.cpp @@ -1323,7 +1323,8 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) int wmId, wmEvent; PAINTSTRUCT ps; HDC hdc; - TCHAR buffer[1000]; + const size_t bufferLen = 1000; + TCHAR buffer[bufferLen]; int SelfIndex; int ReturnValue; HMENU SelfMenu; @@ -1556,7 +1557,11 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) if(FindResult != LB_ERR) { //it was found, get the text, modify text delete it from list, add modified to list - SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, FindResult, reinterpret_cast(buffer)); + auto lbTextLen = ::SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXTLEN, FindResult, 0); + if (lbTextLen > bufferLen) + return TRUE; + + SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, FindResult, reinterpret_cast(buffer)); if((BOOL)lParam) { buffer[10] = 'P'; @@ -1679,6 +1684,9 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) int j = static_cast(SendMessage(BGHS[SelfIndex].hlist1, LB_GETCOUNT, 0, 0)); if(j>0) { + auto lbTextLen = ::SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXTLEN, j-1, 0); + if (lbTextLen > bufferLen) + return TRUE; SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, j - 1, reinterpret_cast(buffer)); buffer[5]=0x00; j=generic_atoi(buffer); @@ -1779,6 +1787,9 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) if(FindResult != LB_ERR) { //it was found, get it + auto lbTextLen = ::SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXTLEN, FindResult, 0); + if (lbTextLen > bufferLen) + return TRUE; SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, FindResult, reinterpret_cast(buffer)); switch (buffer[10]) // no need to call BGM_GETPROTECTION separately for this { @@ -1935,6 +1946,9 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) if(FindResult != LB_ERR) { //it was found, get it + auto lbTextLen = ::SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXTLEN, FindResult, 0); + if (lbTextLen > bufferLen) + return TRUE; SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, FindResult, reinterpret_cast(buffer)); switch (buffer[11]) { @@ -1964,6 +1978,9 @@ LRESULT CALLBACK GridProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) if(FindResult != LB_ERR) { //it was found, get it + auto lbTextLen = ::SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXTLEN, FindResult, 0); + if (lbTextLen > bufferLen) + return TRUE; SendMessage(BGHS[SelfIndex].hlist1, LB_GETTEXT, FindResult, reinterpret_cast(buffer)); switch (buffer[10]) { @@ -3149,9 +3166,10 @@ int BinarySearchListBox(HWND lbhWnd,TCHAR* searchtext) int lbcount; int head,tail,finger; int FindResult; - TCHAR tbuffer[1000]; - TCHAR headtext[1000]; - TCHAR tailtext[1000]; + const size_t bufLen = 1000; + TCHAR tbuffer[bufLen]; + TCHAR headtext[bufLen]; + TCHAR tailtext[bufLen]; int p; BOOL FOUND; @@ -3176,6 +3194,10 @@ int BinarySearchListBox(HWND lbhWnd,TCHAR* searchtext) tail = lbcount - 1; //is it the head? + auto lbTextLen = ::SendMessage(lbhWnd, LB_GETTEXTLEN, head, 0); + if (lbTextLen > bufLen) + return 0; + SendMessage(lbhWnd, LB_GETTEXT, head, reinterpret_cast(headtext)); headtext[9] = 0x00; @@ -3196,6 +3218,9 @@ int BinarySearchListBox(HWND lbhWnd,TCHAR* searchtext) //is it the tail? + lbTextLen = ::SendMessage(lbhWnd, LB_GETTEXTLEN, tail, 0); + if (lbTextLen > bufLen) + return 0; SendMessage(lbhWnd, LB_GETTEXT, tail, reinterpret_cast(tailtext)); tailtext[9] = 0x00; p=lstrcmp(searchtext,tailtext); @@ -3220,7 +3245,9 @@ int BinarySearchListBox(HWND lbhWnd,TCHAR* searchtext) while((!FOUND)&&((tail-head)>1)) { finger = head + ((tail - head) / 2); - + lbTextLen = ::SendMessage(lbhWnd, LB_GETTEXTLEN, finger, 0); + if (lbTextLen > bufLen) + return 0; SendMessage(lbhWnd, LB_GETTEXT, finger, reinterpret_cast(tbuffer)); tbuffer[9] = 0x00; p=lstrcmp(tbuffer,searchtext); diff --git a/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp b/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp index 8192262b..ab094970 100644 --- a/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp +++ b/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp @@ -258,12 +258,19 @@ int32_t PreferenceDlg::getIndexFromName(const TCHAR *name) const return -1; } -void PreferenceDlg::setListSelection(size_t currentSel) const +bool PreferenceDlg::setListSelection(size_t currentSel) const { // Stupid LB API doesn't allow LB_SETSEL to be used on single select listbox, so we do it in a hard way - TCHAR selStr[256]; + const size_t selStrLenMax = 255; + TCHAR selStr[selStrLenMax + 1]; + auto lbTextLen = ::SendMessage(_hSelf, LB_GETTEXTLEN, currentSel, 0); + + if (lbTextLen > selStrLenMax) + return false; + ::SendDlgItemMessage(_hSelf, IDC_LIST_DLGTITLE, LB_GETTEXT, currentSel, reinterpret_cast(selStr)); ::SendDlgItemMessage(_hSelf, IDC_LIST_DLGTITLE, LB_SELECTSTRING, currentSel, reinterpret_cast(selStr)); + return true; } bool PreferenceDlg::renameDialogTitle(const TCHAR *internalName, const TCHAR *newName) @@ -1755,7 +1762,12 @@ INT_PTR CALLBACK LangMenuDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM lP if (iRemove == -1) return TRUE; - TCHAR s[32]; + const size_t sL = 31; + TCHAR s[sL + 1]; + auto lbTextLen = ::SendDlgItemMessage(_hSelf, list2Remove, LB_GETTEXTLEN, iRemove, 0); + if (lbTextLen > sL) + return TRUE; + ::SendDlgItemMessage(_hSelf, list2Remove, LB_GETTEXT, iRemove, reinterpret_cast(s)); LangMenuItem lmi = pSrcLst->at(iRemove); diff --git a/PowerEditor/src/WinControls/Preference/preferenceDlg.h b/PowerEditor/src/WinControls/Preference/preferenceDlg.h index 134ca955..d0d975b6 100644 --- a/PowerEditor/src/WinControls/Preference/preferenceDlg.h +++ b/PowerEditor/src/WinControls/Preference/preferenceDlg.h @@ -249,7 +249,7 @@ public : }; void showDialogByName(const TCHAR *name) const; - void setListSelection(size_t currentSel) const; + bool setListSelection(size_t currentSel) const; virtual void destroy();