https://git.reactos.org/?p=reactos.git;a=commitdiff;h=222acf5a3ed11f74adbe0d1bdee527447fd81bbf
commit 222acf5a3ed11f74adbe0d1bdee527447fd81bbf Author: Joachim Henze <[email protected]> AuthorDate: Mon Sep 20 03:05:05 2021 +0200 Commit: Joachim Henze <[email protected]> CommitDate: Mon Sep 20 03:05:05 2021 +0200 [NTUSER] Scrollbar.c, Avoid potential out-of-bounds-accesses in co_IntSetScrollInfo() CORE-17777 This is an addendum to 0.4.15-dev-3174-g dda9c3979e87223b66e61f0f936f46920221e509 CORE-17769 and 0.4.15-dev-3147-g 3bf7e3ac13c5c9df373827c102b763b5b9822204 CORE-17754 CORE-17755 We have not seen this happening in real-life yet, but some code-fragments within co_IntSetScrollInfo() e.g. line 628 if (nBar == SB_CTL) do clearly indicate that nBar can be 2 (SB_CTL). Some lines below we definitely must not access those 4 static arrays out of bounds then via nBar as access index! Ftr with a bit of grepping I also found some calls like NtUserSetScrollInfo(Wnd, SB_CTL, &Info, FALSE); e.g: in win32ss/user/user32/controls/scrollbar.c so I am pretty sure nBar == 2 can happen in practice within co_IntSetScrollInfo(). I question whether any of those reads/writes to those static arrays (or the comparisons) would make any sense on index 2, so we should aim to eliminate them altogether in the future. --- win32ss/user/ntuser/scrollbar.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/win32ss/user/ntuser/scrollbar.c b/win32ss/user/ntuser/scrollbar.c index eba1b41f666..e4e0ba436af 100644 --- a/win32ss/user/ntuser/scrollbar.c +++ b/win32ss/user/ntuser/scrollbar.c @@ -492,15 +492,15 @@ co_IntSetScrollInfo(PWND Window, INT nBar, LPCSCROLLINFO lpsi, BOOL bRedraw) BOOL bChangeParams = FALSE; /* Don't show/hide scrollbar if params don't change */ UINT MaxPage; int MaxPos; - /* [0] = HORZ, [1] = VERT */ - static PWND PrevHwnd[2] = { 0 }; - static DWORD PrevPos[2] = { 0 }; - static DWORD PrevMax[2] = { 0 }; - static INT PrevAction[2] = { 0 }; + /* [0] = SB_HORZ, [1] = SB_VERT, [2] = SB_CTL */ + static PWND PrevHwnd[3] = { 0 }; + static DWORD PrevPos[3] = { 0 }; + static DWORD PrevMax[3] = { 0 }; + static INT PrevAction[3] = { 0 }; ASSERT_REFS_CO(Window); - if(!SBID_IS_VALID(nBar)) + if(!SBID_IS_VALID(nBar)) /* Assures nBar is 0, 1, or 2 */ { EngSetLastError(ERROR_INVALID_PARAMETER); ERR("Trying to set scrollinfo for unknown scrollbar type %d", nBar);
