[PATCH 0/1] MR6169: comctl32: fixed first we check index then array access by index is safe

[Herman Semenov (＠GermanAizek)]

I think this PR changes will protect against situation if out-of-bounds access array.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6169

[PATCH 1/1] comctl32: fixed first we check index then array access by index is safe

[Herman Semenov]

From: Herman Semenov <GermanAizek(a)yandex.ru>

---
 dlls/comctl32/toolbar.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c
index 690a02db6ee..dddf96ceb93 100644
--- a/dlls/comctl32/toolbar.c
+++ b/dlls/comctl32/toolbar.c
@@ -3240,11 +3240,12 @@ static LRESULT
 TOOLBAR_DeleteButton (TOOLBAR_INFO *infoPtr, INT nIndex)
 {
     NMTOOLBARW nmtb;
-    TBUTTON_INFO *btnPtr = &infoPtr->buttons[nIndex];
 
     if ((nIndex < 0) || (nIndex >= infoPtr->nNumButtons))
         return FALSE;
 
+    TBUTTON_INFO *btnPtr = &infoPtr->buttons[nIndex];
+
     memset(&nmtb, 0, sizeof(nmtb));
     nmtb.iItem = btnPtr->idCommand;
     nmtb.tbButton.iBitmap = btnPtr->iBitmap;
@@ -3545,14 +3546,15 @@ TOOLBAR_GetItemRect (const TOOLBAR_INFO *infoPtr, INT nIndex, LPRECT lpRect)
 {
     TBUTTON_INFO *btnPtr;
 
-    btnPtr = &infoPtr->buttons[nIndex];
     if ((nIndex < 0) || (nIndex >= infoPtr->nNumButtons))
-	return FALSE;
+        return FALSE;
+
+    btnPtr = &infoPtr->buttons[nIndex];
 
     if (lpRect == NULL)
-	return FALSE;
+	    return FALSE;
     if (btnPtr->fsState & TBSTATE_HIDDEN)
-	return FALSE;
+	    return FALSE;
 
     lpRect->left   = btnPtr->rect.left;
     lpRect->right  = btnPtr->rect.right;
@@ -3596,12 +3598,14 @@ TOOLBAR_GetRect (const TOOLBAR_INFO *infoPtr, INT Id, LPRECT lpRect)
     INT        nIndex;
 
     nIndex = TOOLBAR_GetButtonIndex (infoPtr, Id, FALSE);
-    btnPtr = &infoPtr->buttons[nIndex];
+
     if ((nIndex < 0) || (nIndex >= infoPtr->nNumButtons))
-	return FALSE;
+        return FALSE;
+
+    btnPtr = &infoPtr->buttons[nIndex];
 
     if (lpRect == NULL)
-	return FALSE;
+	    return FALSE;
 
     lpRect->left   = btnPtr->rect.left;
     lpRect->right  = btnPtr->rect.right;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6169

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/toolbar.c:
> {
>      TBUTTON_INFO *btnPtr;
>
> -    btnPtr = &infoPtr->buttons[nIndex];
>      if ((nIndex < 0) || (nIndex >= infoPtr->nNumButtons))
> -	return FALSE;
> +        return FALSE;
> +
> +    btnPtr = &infoPtr->buttons[nIndex];
>
> if (lpRect == NULL)
> -	return FALSE;
> +	    return FALSE;
> Please avoid unnecessary whitespace changes.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6169#note_77257