From 58037e07b1e21ba388fddb3dce001e3cce498bd8 Mon Sep 17 00:00:00 2001 From: Don HO Date: Sat, 9 Feb 2019 03:28:52 +0100 Subject: [PATCH] [EU-FOSSA] Fix stack buffer overflow on wsprintf in WordStyle dialog Also remove dynamic allocation for CB_GETLBTEXT and use local array instead by controlling buffer size. --- .../src/ScitillaComponent/FindReplaceDlg.cpp | 11 ++++---- .../ScitillaComponent/UserDefineDialog.cpp | 25 +++++++++++++------ .../WinControls/ColourPicker/WordStyleDlg.cpp | 14 ++++++++--- .../WinControls/Preference/preferenceDlg.cpp | 18 ++++++++----- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp index f3509655..c7fe3d6a 100644 --- a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp +++ b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp @@ -401,6 +401,7 @@ void FindReplaceDlg::saveFindHistory() int FindReplaceDlg::saveComboHistory(int id, int maxcount, vector & strings, bool saveEmpty) { + TCHAR text[FINDREPLACE_MAXLENGTH]; HWND hCombo = ::GetDlgItem(_hSelf, id); int count = static_cast(::SendMessage(hCombo, CB_GETCOUNT, 0, 0)); count = min(count, maxcount); @@ -421,11 +422,11 @@ int FindReplaceDlg::saveComboHistory(int id, int maxcount, vector(text)); - strings.push_back(generic_string(text)); - delete[] text; + if (cbTextLen <= FINDREPLACE_MAXLENGTH - 1) + { + ::SendMessage(hCombo, CB_GETLBTEXT, i, reinterpret_cast(text)); + strings.push_back(generic_string(text)); + } } return count; } diff --git a/PowerEditor/src/ScitillaComponent/UserDefineDialog.cpp b/PowerEditor/src/ScitillaComponent/UserDefineDialog.cpp index e2d24a0c..38b769c9 100644 --- a/PowerEditor/src/ScitillaComponent/UserDefineDialog.cpp +++ b/PowerEditor/src/ScitillaComponent/UserDefineDialog.cpp @@ -1139,8 +1139,12 @@ INT_PTR CALLBACK UserDefineDialog::run_dlgProc(UINT message, WPARAM wParam, LPAR if (result == IDYES) { auto i = ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETCURSEL, 0, 0); - auto cbTextLen = ::SendMessage(_hSelf, CB_GETLBTEXTLEN, i, 0); - TCHAR * langName = new TCHAR[cbTextLen + 1]; + const size_t langNameLen = 256; + TCHAR langName[langNameLen + 1]; + auto cbTextLen = ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETLBTEXTLEN, i, 0); + if (cbTextLen > langNameLen) + return TRUE; + ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETLBTEXT, i, reinterpret_cast(langName)); //remove current language from combobox @@ -1158,16 +1162,18 @@ INT_PTR CALLBACK UserDefineDialog::run_dlgProc(UINT message, WPARAM wParam, LPAR ::RemoveMenu(subMenu, static_cast(IDM_LANG_USER + i), MF_BYCOMMAND); ::DrawMenuBar(hNpp); ::SendMessage(_hParent, WM_REMOVE_USERLANG, 0, reinterpret_cast(langName)); - - delete[] langName; } return TRUE; } case IDC_RENAME_BUTTON : { auto i = ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETCURSEL, 0, 0); + const size_t langNameLen = 256; + TCHAR langName[langNameLen + 1]; auto cbTextLen = ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETLBTEXTLEN, i, 0); - TCHAR * langName = new TCHAR[cbTextLen + 1]; + if (cbTextLen > langNameLen) + return TRUE; + ::SendDlgItemMessage(_hSelf, IDC_LANGNAME_COMBO, CB_GETLBTEXT, i, reinterpret_cast(langName)); StringDlg strDlg; @@ -1203,8 +1209,6 @@ INT_PTR CALLBACK UserDefineDialog::run_dlgProc(UINT message, WPARAM wParam, LPAR ::ModifyMenu(hSubM, static_cast(IDM_LANG_USER + i), MF_BYCOMMAND, IDM_LANG_USER + i, newName); ::DrawMenuBar(hNpp); ::SendMessage(_hParent, WM_RENAME_USERLANG, reinterpret_cast(newName), reinterpret_cast(langName)); - - delete[] langName; } return TRUE; @@ -1589,9 +1593,14 @@ INT_PTR CALLBACK StylerDlg::dlgProc(HWND hwnd, UINT message, WPARAM wParam, LPAR auto i = ::SendDlgItemMessage(hwnd, LOWORD(wParam), CB_GETCURSEL, 0, 0); if (LOWORD(wParam) == IDC_STYLER_COMBO_FONT_SIZE) { - TCHAR intStr[32]; if (i != 0) { + const size_t intStrLen = 3; + TCHAR intStr[intStrLen]; + auto lbTextLen = ::SendDlgItemMessage(hwnd, LOWORD(wParam), CB_GETLBTEXTLEN, i, 0); + if (lbTextLen > intStrLen - 1) + return TRUE; + ::SendDlgItemMessage(hwnd, LOWORD(wParam), CB_GETLBTEXT, i, reinterpret_cast(intStr)); if ((!intStr) || (!intStr[0])) style._fontSize = -1; diff --git a/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp b/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp index 5555fef3..8651097d 100644 --- a/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp +++ b/PowerEditor/src/WinControls/ColourPicker/WordStyleDlg.cpp @@ -543,10 +543,17 @@ void WordStyleDlg::updateFontSize() Style & style = getCurrentStyler(); auto iFontSizeSel = ::SendMessage(_hFontSizeCombo, CB_GETCURSEL, 0, 0); - TCHAR intStr[32]; if (iFontSizeSel != 0) { + const size_t intStrLen = 3; + TCHAR intStr[intStrLen]; + + auto lbTextLen = ::SendMessage(_hFontSizeCombo, CB_GETLBTEXTLEN, iFontSizeSel, 0); + if (lbTextLen >= intStrLen) + return; + ::SendMessage(_hFontSizeCombo, CB_GETLBTEXT, iFontSizeSel, reinterpret_cast(intStr)); + if (!intStr[0]) style._fontSize = STYLE_NOT_USED; else @@ -783,9 +790,10 @@ void WordStyleDlg::setVisualFromStyleList() //-- font size isEnable = false; - TCHAR intStr[5] = TEXT(""); + const size_t intStrLen = 3; + TCHAR intStr[intStrLen]; LRESULT iFontSize = 0; - if (style._fontSize != STYLE_NOT_USED) + if (style._fontSize != STYLE_NOT_USED && style._fontSize < 100) // style._fontSize has only 2 digits { wsprintf(intStr, TEXT("%d"), style._fontSize); iFontSize = ::SendMessage(_hFontSizeCombo, CB_FINDSTRING, 1, reinterpret_cast(intStr)); diff --git a/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp b/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp index ab094970..f18eed88 100644 --- a/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp +++ b/PowerEditor/src/WinControls/Preference/preferenceDlg.cpp @@ -539,12 +539,14 @@ INT_PTR CALLBACK BarsDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM) { LocalizationSwitcher & localizationSwitcher = pNppParam->getLocalizationSwitcher(); auto index = ::SendDlgItemMessage(_hSelf, IDC_COMBO_LOCALIZATION, CB_GETCURSEL, 0, 0); - auto cbTextLen = ::SendMessage(_hSelf, CB_GETLBTEXTLEN, index, 0); - TCHAR * langName = new TCHAR[cbTextLen + 1]; + TCHAR langName[MAX_PATH]; + auto cbTextLen = ::SendDlgItemMessage(_hSelf, IDC_COMBO_LOCALIZATION, CB_GETLBTEXTLEN, index, 0); + if (cbTextLen > MAX_PATH - 1) + return TRUE; + ::SendDlgItemMessage(_hSelf, IDC_COMBO_LOCALIZATION, CB_GETLBTEXT, index, reinterpret_cast(langName)); if (langName[0]) { - // Make English as basic language if (localizationSwitcher.switchToLang(TEXT("English"))) { @@ -557,8 +559,6 @@ INT_PTR CALLBACK BarsDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM) ::InvalidateRect(_hParent, NULL, TRUE); } } - - delete[] langName; } return TRUE; default: @@ -2321,7 +2321,13 @@ INT_PTR CALLBACK PrintSettingsDlg::run_dlgProc(UINT message, WPARAM wParam, LPAR case IDC_COMBO_HFONTSIZE : case IDC_COMBO_FFONTSIZE : { - TCHAR intStr[32]; + const size_t intStrLen = 3; + TCHAR intStr[intStrLen]; + + auto lbTextLen = ::SendDlgItemMessage(_hSelf, LOWORD(wParam), CB_GETLBTEXTLEN, iSel, 0); + if (lbTextLen >= intStrLen) + return TRUE; + ::SendDlgItemMessage(_hSelf, LOWORD(wParam), CB_GETLBTEXT, iSel, reinterpret_cast(intStr)); int *pVal = (LOWORD(wParam) == IDC_COMBO_HFONTSIZE)?&(nppGUI._printSettings._headerFontSize):&(nppGUI._printSettings._footerFontSize);