[ros-diffs] [reactos] 01/01: [NOTEPAD] Avoid half-initialized status of settings (#5078)

[Katayama Hirofumi MZ]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=ead9366ef79747a9c2ea1…

commit ead9366ef79747a9c2ea12492de6688f34005fb2
Author:     Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com>
AuthorDate: Tue Feb 21 09:22:41 2023 +0900
Commit:     GitHub <noreply(a)github.com>
CommitDate: Tue Feb 21 09:22:41 2023 +0900

    [NOTEPAD] Avoid half-initialized status of settings (#5078)
    
    - Avoid buffer overrun in QueryString helper function.
    - Improve NOTEPAD_LoadSettingsFromRegistry function.
    CORE-18837
---
 base/applications/notepad/main.c     |   6 +-
 base/applications/notepad/settings.c | 129 ++++++++++++++++++-----------------
 2 files changed, 67 insertions(+), 68 deletions(-)

diff --git a/base/applications/notepad/main.c b/base/applications/notepad/main.c
index f99fb40621a..e42b0c749e0 100644
--- a/base/applications/notepad/main.c
+++ b/base/applications/notepad/main.c
@@ -560,6 +560,7 @@ int WINAPI _tWinMain(HINSTANCE hInstance, HINSTANCE prev, LPTSTR
cmdline, int sh
     HMONITOR monitor;
     MONITORINFO info;
     INT x, y;
+    RECT rcIntersect;
 
     static const TCHAR className[] = _T("Notepad");
     static const TCHAR winName[] = _T("Notepad");
@@ -609,10 +610,7 @@ int WINAPI _tWinMain(HINSTANCE hInstance, HINSTANCE prev, LPTSTR
cmdline, int sh
 
     x = Globals.main_rect.left;
     y = Globals.main_rect.top;
-    if (Globals.main_rect.left >= info.rcWork.right ||
-        Globals.main_rect.top >= info.rcWork.bottom ||
-        Globals.main_rect.right < info.rcWork.left ||
-        Globals.main_rect.bottom < info.rcWork.top)
+    if (!IntersectRect(&rcIntersect, &Globals.main_rect, &info.rcWork))
         x = y = CW_USEDEFAULT;
 
     Globals.hMainWnd = CreateWindow(className,
diff --git a/base/applications/notepad/settings.c b/base/applications/notepad/settings.c
index 0031e14f9b0..32bfd098683 100644
--- a/base/applications/notepad/settings.c
+++ b/base/applications/notepad/settings.c
@@ -100,9 +100,14 @@ static BOOL QueryBool(HKEY hKey, LPCTSTR pszValueName, BOOL
*pbResult)
     return TRUE;
 }
 
-static BOOL QueryString(HKEY hKey, LPCTSTR pszValueName, LPTSTR pszResult, DWORD
dwResultSize)
+static BOOL QueryString(HKEY hKey, LPCTSTR pszValueName, LPTSTR pszResult, DWORD
dwResultLength)
 {
-    return QueryGeneric(hKey, pszValueName, REG_SZ, pszResult, dwResultSize *
sizeof(TCHAR));
+    if (dwResultLength == 0)
+        return FALSE;
+    if (!QueryGeneric(hKey, pszValueName, REG_SZ, pszResult, dwResultLength *
sizeof(TCHAR)))
+        return FALSE;
+    pszResult[dwResultLength - 1] = 0; /* Avoid buffer overrun */
+    return TRUE;
 }
 
 /***********************************************************************
@@ -113,24 +118,38 @@ static BOOL QueryString(HKEY hKey, LPCTSTR pszValueName, LPTSTR
pszResult, DWORD
  */
 void NOTEPAD_LoadSettingsFromRegistry(void)
 {
-    HKEY hKey = NULL;
+    HKEY hKey;
     HFONT hFont;
-    DWORD dwPointSize = 0;
-    INT base_length, dx, dy;
-
-    base_length = (GetSystemMetrics(SM_CXSCREEN) > GetSystemMetrics(SM_CYSCREEN)) ?
-                  GetSystemMetrics(SM_CYSCREEN) : GetSystemMetrics(SM_CXSCREEN);
-
-    dx = (INT)(base_length * .95);
-    dy = dx * 3 / 4;
-    SetRect(&Globals.main_rect, 0, 0, dx, dy);
-
-    if (RegOpenKey(HKEY_CURRENT_USER, s_szRegistryKey, &hKey) == ERROR_SUCCESS)
+    DWORD dwPointSize, cx, cy;
+    DWORD cxScreen = GetSystemMetrics(SM_CXSCREEN), cyScreen =
GetSystemMetrics(SM_CYSCREEN);
+
+    /* Set the default values */
+    Globals.bShowStatusBar = TRUE;
+    Globals.bWrapLongLines = FALSE;
+    SetRect(&Globals.lMargins, 750, 1000, 750, 1000);
+    ZeroMemory(&Globals.lfFont, sizeof(Globals.lfFont));
+    Globals.lfFont.lfCharSet = DEFAULT_CHARSET;
+    dwPointSize = 100;
+    Globals.lfFont.lfWeight = FW_NORMAL;
+    Globals.lfFont.lfPitchAndFamily = FIXED_PITCH | FF_MODERN;
+    Globals.main_rect.left = CW_USEDEFAULT;
+    Globals.main_rect.top = CW_USEDEFAULT;
+    cx = min((cxScreen * 3) / 4, 640);
+    cy = min((cyScreen * 3) / 4, 480);
+
+    /* FIXME: Globals.fSaveWindowPositions = FALSE; */
+    /* FIXME: Globals.fMLE_is_broken = FALSE; */
+
+    /* Open the target registry key */
+    if (RegOpenKey(HKEY_CURRENT_USER, s_szRegistryKey, &hKey) != ERROR_SUCCESS)
+        hKey = NULL;
+
+    /* Load the values from registry */
+    if (hKey)
     {
         QueryByte(hKey, _T("lfCharSet"), &Globals.lfFont.lfCharSet);
         QueryByte(hKey, _T("lfClipPrecision"),
&Globals.lfFont.lfClipPrecision);
         QueryDword(hKey, _T("lfEscapement"),
(DWORD*)&Globals.lfFont.lfEscapement);
-        QueryString(hKey, _T("lfFaceName"), Globals.lfFont.lfFaceName,
ARRAY_SIZE(Globals.lfFont.lfFaceName));
         QueryByte(hKey, _T("lfItalic"), &Globals.lfFont.lfItalic);
         QueryDword(hKey, _T("lfOrientation"),
(DWORD*)&Globals.lfFont.lfOrientation);
         QueryByte(hKey, _T("lfOutPrecision"),
&Globals.lfFont.lfOutPrecision);
@@ -140,10 +159,10 @@ void NOTEPAD_LoadSettingsFromRegistry(void)
         QueryByte(hKey, _T("lfUnderline"), &Globals.lfFont.lfUnderline);
         QueryDword(hKey, _T("lfWeight"),
(DWORD*)&Globals.lfFont.lfWeight);
         QueryDword(hKey, _T("iPointSize"), &dwPointSize);
+
         QueryBool(hKey, _T("fWrap"), &Globals.bWrapLongLines);
         QueryBool(hKey, _T("fStatusBar"), &Globals.bShowStatusBar);
-        QueryString(hKey, _T("szHeader"), Globals.szHeader,
ARRAY_SIZE(Globals.szHeader));
-        QueryString(hKey, _T("szTrailer"), Globals.szFooter,
ARRAY_SIZE(Globals.szFooter));
+
         QueryDword(hKey, _T("iMarginLeft"),
(DWORD*)&Globals.lMargins.left);
         QueryDword(hKey, _T("iMarginTop"), (DWORD*)&Globals.lMargins.top);
         QueryDword(hKey, _T("iMarginRight"),
(DWORD*)&Globals.lMargins.right);
@@ -151,62 +170,44 @@ void NOTEPAD_LoadSettingsFromRegistry(void)
 
         QueryDword(hKey, _T("iWindowPosX"),
(DWORD*)&Globals.main_rect.left);
         QueryDword(hKey, _T("iWindowPosY"),
(DWORD*)&Globals.main_rect.top);
-        QueryDword(hKey, _T("iWindowPosDX"), (DWORD*)&dx);
-        QueryDword(hKey, _T("iWindowPosDY"), (DWORD*)&dy);
-
-        Globals.main_rect.right = Globals.main_rect.left + dx;
-        Globals.main_rect.bottom = Globals.main_rect.top + dy;
+        QueryDword(hKey, _T("iWindowPosDX"), &cx);
+        QueryDword(hKey, _T("iWindowPosDY"), &cy);
+    }
 
-        if (dwPointSize != 0)
-            Globals.lfFont.lfHeight = HeightFromPointSize(dwPointSize);
-        else
-            Globals.lfFont.lfHeight = HeightFromPointSize(100);
+    Globals.lfFont.lfHeight = HeightFromPointSize(dwPointSize);
+    Globals.main_rect.right = Globals.main_rect.left + cx;
+    Globals.main_rect.bottom = Globals.main_rect.top + cy;
 
-        RegCloseKey(hKey);
-    }
-    else
+    if (!hKey || !QueryString(hKey, _T("lfFaceName"),
+                              Globals.lfFont.lfFaceName,
ARRAY_SIZE(Globals.lfFont.lfFaceName)))
     {
-        /* If no settings are found in the registry, then use default values */
-        Globals.bShowStatusBar = TRUE;
-        Globals.bWrapLongLines = FALSE;
-        SetRect(&Globals.lMargins, 750, 1000, 750, 1000);
-
-        /* FIXME: Globals.fSaveWindowPositions = FALSE; */
-        /* FIXME: Globals.fMLE_is_broken = FALSE; */
+        LoadString(Globals.hInstance, STRING_DEFAULTFONT, Globals.lfFont.lfFaceName,
+                   ARRAY_SIZE(Globals.lfFont.lfFaceName));
+    }
 
+    if (!hKey || !QueryString(hKey, _T("szHeader"), Globals.szHeader,
ARRAY_SIZE(Globals.szHeader)))
+    {
         LoadString(Globals.hInstance, STRING_PAGESETUP_HEADERVALUE, Globals.szHeader,
                    ARRAY_SIZE(Globals.szHeader));
+    }
+
+    if (!hKey || !QueryString(hKey, _T("szTrailer"), Globals.szFooter,
ARRAY_SIZE(Globals.szFooter)))
+    {
         LoadString(Globals.hInstance, STRING_PAGESETUP_FOOTERVALUE, Globals.szFooter,
                    ARRAY_SIZE(Globals.szFooter));
+    }
 
-        ZeroMemory(&Globals.lfFont, sizeof(Globals.lfFont));
-        Globals.lfFont.lfCharSet = DEFAULT_CHARSET;
-        Globals.lfFont.lfClipPrecision = CLIP_STROKE_PRECIS;
-        Globals.lfFont.lfEscapement = 0;
-        LoadString(Globals.hInstance, STRING_DEFAULTFONT, Globals.lfFont.lfFaceName,
-                   ARRAY_SIZE(Globals.lfFont.lfFaceName));
-        Globals.lfFont.lfItalic = FALSE;
-        Globals.lfFont.lfOrientation = 0;
-        Globals.lfFont.lfOutPrecision = OUT_STRING_PRECIS;
-
-        /* WORKAROUND: Far East Asian users may not have suitable fixed-pitch fonts. */
-        switch (PRIMARYLANGID(GetUserDefaultLangID()))
-        {
-            case LANG_CHINESE:
-            case LANG_JAPANESE:
-            case LANG_KOREAN:
-                Globals.lfFont.lfPitchAndFamily = DEFAULT_PITCH | FF_DONTCARE;
-                break;
-            default:
-                Globals.lfFont.lfPitchAndFamily = FIXED_PITCH | FF_MODERN;
-                break;
-        }
-
-        Globals.lfFont.lfQuality = PROOF_QUALITY;
-        Globals.lfFont.lfStrikeOut = FALSE;
-        Globals.lfFont.lfUnderline = FALSE;
-        Globals.lfFont.lfWeight = FW_NORMAL;
-        Globals.lfFont.lfHeight = HeightFromPointSize(100);
+    if (hKey)
+        RegCloseKey(hKey);
+
+    /* WORKAROUND: Far East Asian users may not have suitable fixed-pitch fonts. */
+    switch (PRIMARYLANGID(GetUserDefaultLangID()))
+    {
+        case LANG_CHINESE:
+        case LANG_JAPANESE:
+        case LANG_KOREAN:
+            Globals.lfFont.lfPitchAndFamily = DEFAULT_PITCH | FF_DONTCARE;
+            break;
     }
 
     hFont = CreateFontIndirect(&Globals.lfFont);