From fe261bd6e233c39237302fbbd23794bc9a553eac Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Thu, 18 Dec 2014 08:12:41 +0000 Subject: [PATCH] [WIN32K] - Make REGION_iOffsetRgn check the region for coordinate space overflow and fail, if the region cannot be moved - Rename REGION_iOffsetRgn to REGION_bOffsetRgn and make it return BOOL instead of the complexity, since the majority of callers are not interested in the complexity. It's also more obvious that we need to check for an error. svn path=/trunk/; revision=65733 --- reactos/win32ss/gdi/ntgdi/bitblt.c | 2 +- reactos/win32ss/gdi/ntgdi/cliprgn.c | 20 ++-- reactos/win32ss/gdi/ntgdi/coord.h | 4 + reactos/win32ss/gdi/ntgdi/dcobjs.c | 5 +- reactos/win32ss/gdi/ntgdi/region.c | 128 ++++++++++++++++++------- reactos/win32ss/gdi/ntgdi/region.h | 8 +- reactos/win32ss/user/ntuser/painting.c | 14 +-- reactos/win32ss/user/ntuser/vis.c | 14 +-- reactos/win32ss/user/ntuser/windc.c | 2 +- reactos/win32ss/user/ntuser/winpos.c | 16 ++-- 10 files changed, 147 insertions(+), 66 deletions(-) diff --git a/reactos/win32ss/gdi/ntgdi/bitblt.c b/reactos/win32ss/gdi/ntgdi/bitblt.c index a408509234d..760c2457d63 100644 --- a/reactos/win32ss/gdi/ntgdi/bitblt.c +++ b/reactos/win32ss/gdi/ntgdi/bitblt.c @@ -1053,7 +1053,7 @@ IntGdiFillRgn( /* Transform region into device coordinates */ if (!REGION_LPTODP(pdc, prgnClip, prgn) || - REGION_iOffsetRgn(prgnClip, pdc->ptlDCOrig.x, pdc->ptlDCOrig.y) == ERROR) + !REGION_bOffsetRgn(prgnClip, pdc->ptlDCOrig.x, pdc->ptlDCOrig.y)) { REGION_Delete(prgnClip); return FALSE; diff --git a/reactos/win32ss/gdi/ntgdi/cliprgn.c b/reactos/win32ss/gdi/ntgdi/cliprgn.c index e1627ed26c4..9f8b87e5a29 100644 --- a/reactos/win32ss/gdi/ntgdi/cliprgn.c +++ b/reactos/win32ss/gdi/ntgdi/cliprgn.c @@ -57,7 +57,7 @@ GdiSelectVisRgn( ASSERT(prgn != NULL); IntGdiCombineRgn(dc->prgnVis, prgn, NULL, RGN_COPY); - REGION_iOffsetRgn(dc->prgnVis, -dc->ptlDCOrig.x, -dc->ptlDCOrig.y); + REGION_bOffsetRgn(dc->prgnVis, -dc->ptlDCOrig.x, -dc->ptlDCOrig.y); DC_UnlockDc(dc); } @@ -365,9 +365,17 @@ int APIENTRY NtGdiOffsetClipRgn(HDC hDC, if(dc->dclevel.prgnClip != NULL) { - Result = REGION_iOffsetRgn(dc->dclevel.prgnClip, - XOffset, - YOffset); + if (!REGION_bOffsetRgn(dc->dclevel.prgnClip, + XOffset, + YOffset)) + { + Result = ERROR; + } + else + { + Result = REGION_Complexity(dc->dclevel.prgnClip); + } + dc->fs |= DC_FLAG_DIRTY_RAO; } else @@ -572,7 +580,7 @@ CLIPPING_UpdateGCRegion(PDC pDC) } - REGION_iOffsetRgn(pDC->prgnRao, pDC->ptlDCOrig.x, pDC->ptlDCOrig.y); + REGION_bOffsetRgn(pDC->prgnRao, pDC->ptlDCOrig.x, pDC->ptlDCOrig.y); RtlCopyMemory(&pDC->erclClip, &pDC->prgnRao->rdh.rcBound, @@ -590,7 +598,7 @@ CLIPPING_UpdateGCRegion(PDC pDC) pDC->prgnRao->Buffer, &pDC->erclClip); - REGION_iOffsetRgn(pDC->prgnRao, -pDC->ptlDCOrig.x, -pDC->ptlDCOrig.y); + REGION_bOffsetRgn(pDC->prgnRao, -pDC->ptlDCOrig.x, -pDC->ptlDCOrig.y); } /* EOF */ diff --git a/reactos/win32ss/gdi/ntgdi/coord.h b/reactos/win32ss/gdi/ntgdi/coord.h index c433f76cc3d..5e72d57cb39 100644 --- a/reactos/win32ss/gdi/ntgdi/coord.h +++ b/reactos/win32ss/gdi/ntgdi/coord.h @@ -1,5 +1,9 @@ #pragma once +/* Maximum extend of coordinate space */ +#define MIN_COORD (INT_MIN / 16) +#define MAX_COORD (INT_MAX / 16) + #define IntLPtoDP(pdc, ppt, count) DC_vXformWorldToDevice(pdc, count, (PPOINTL)(ppt), (PPOINTL)(ppt)); #define CoordLPtoDP(pdc, ppt) DC_vXformWorldToDevice(pdc, 1, (PPOINTL)(ppt), (PPOINTL)(ppt)); #define IntDPtoLP(pdc, ppt, count) DC_vXformDeviceToWorld(pdc, count, (PPOINTL)(ppt), (PPOINTL)(ppt)); diff --git a/reactos/win32ss/gdi/ntgdi/dcobjs.c b/reactos/win32ss/gdi/ntgdi/dcobjs.c index 6caeccd077c..15448670e3a 100644 --- a/reactos/win32ss/gdi/ntgdi/dcobjs.c +++ b/reactos/win32ss/gdi/ntgdi/dcobjs.c @@ -779,7 +779,10 @@ NtGdiGetRandomRgn( { ret = IntGdiCombineRgn(prgnDest, prgnSrc, 0, RGN_COPY) == ERROR ? -1 : 1; if ((ret == 1) && (iCode == SYSRGN)) - REGION_iOffsetRgn(prgnDest, pdc->ptlDCOrig.x, pdc->ptlDCOrig.y); + { + /// \todo FIXME This is not really correct, since we already modified the region + ret = REGION_bOffsetRgn(prgnDest, pdc->ptlDCOrig.x, pdc->ptlDCOrig.y); + } REGION_UnlockRgn(prgnDest); } else diff --git a/reactos/win32ss/gdi/ntgdi/region.c b/reactos/win32ss/gdi/ntgdi/region.c index 561faa30073..d56028d9c42 100644 --- a/reactos/win32ss/gdi/ntgdi/region.c +++ b/reactos/win32ss/gdi/ntgdi/region.c @@ -514,7 +514,7 @@ REGION_CopyRegion( PREGION dst, PREGION src) { - /* Only copy if source and dest are equal */ + /* Only copy if source and dest are not equal */ if (dst != src) { /* Check if we need to increase our buffer */ @@ -1867,31 +1867,31 @@ REGION_bMakeFrameRegion( else { /* Move the source region to the bottom-right */ - REGION_iOffsetRgn(prgnSrc, cx, cy); + REGION_bOffsetRgn(prgnSrc, cx, cy); /* Intersect with the source region (this crops the top-left frame) */ REGION_IntersectRegion(prgnDest, prgnDest, prgnSrc); /* Move the source region to the bottom-left */ - REGION_iOffsetRgn(prgnSrc, -2 * cx, 0); + REGION_bOffsetRgn(prgnSrc, -2 * cx, 0); /* Intersect with the source region (this crops the top-right frame) */ REGION_IntersectRegion(prgnDest, prgnDest, prgnSrc); /* Move the source region to the top-left */ - REGION_iOffsetRgn(prgnSrc, 0, -2 * cy); + REGION_bOffsetRgn(prgnSrc, 0, -2 * cy); /* Intersect with the source region (this crops the bottom-right frame) */ REGION_IntersectRegion(prgnDest, prgnDest, prgnSrc); /* Move the source region to the top-right */ - REGION_iOffsetRgn(prgnSrc, 2 * cx, 0); + REGION_bOffsetRgn(prgnSrc, 2 * cx, 0); /* Intersect with the source region (this crops the bottom-left frame) */ REGION_IntersectRegion(prgnDest, prgnDest, prgnSrc); /* Move the source region back to the original position */ - REGION_iOffsetRgn(prgnSrc, -cx, cy); + REGION_bOffsetRgn(prgnSrc, -cx, cy); /* Finally subtract the cropped region from the source */ REGION_SubtractRegion(prgnDest, prgnSrc, prgnDest); @@ -1961,7 +1961,7 @@ REGION_bXformRgn( if (pmx->flAccel & XFORM_UNITY) { /* Just offset the region */ - return REGION_iOffsetRgn(prgn, (pmx->fxDx + 8) / 16, (pmx->fxDy + 8) / 16) != ERROR; + return REGION_bOffsetRgn(prgn, (pmx->fxDx + 8) / 16, (pmx->fxDy + 8) / 16); } else { @@ -2584,40 +2584,93 @@ REGION_SetRectRgn( } } -INT +BOOL FASTCALL -REGION_iOffsetRgn( - PREGION rgn, - INT XOffset, - INT YOffset) +REGION_bOffsetRgn( + _Inout_ PREGION prgn, + _In_ INT cx, + _In_ INT cy) { - if (XOffset || YOffset) + PRECTL prcl; + UINT i; + + NT_ASSERT(prgn != NULL); + + /* Check for trivial case */ + if ((cx == 0) && (cy == 0)) { - int nbox = rgn->rdh.nCount; - PRECTL pbox = rgn->Buffer; + return TRUE; + } - if (nbox && pbox) + /* Check for empty regions, we ignore the offset values here */ + if (prgn->rdh.nCount == 0) + { + return TRUE; + } + + /* Make sure the offset is within the legal range */ + if ((cx > MAX_COORD) || (cx < MIN_COORD) || + (cy > MAX_COORD) || (cy < MIN_COORD)) + { + return FALSE; + } + + /* Are we moving right? */ + if (cx > 0) + { + /* Check if we stay inside the bounds on the right side */ + if (prgn->rdh.rcBound.right > (MAX_COORD - cx)) { - while (nbox--) - { - pbox->left += XOffset; - pbox->right += XOffset; - pbox->top += YOffset; - pbox->bottom += YOffset; - pbox++; - } - - if (rgn->Buffer != &rgn->rdh.rcBound) - { - rgn->rdh.rcBound.left += XOffset; - rgn->rdh.rcBound.right += XOffset; - rgn->rdh.rcBound.top += YOffset; - rgn->rdh.rcBound.bottom += YOffset; - } + return FALSE; + } + } + else + { + /* Check if we stay inside the bounds on the left side */ + if (prgn->rdh.rcBound.left < (MIN_COORD - cx)) + { + return FALSE; } } - return REGION_Complexity(rgn); + /* Are we moving down? */ + if (cy > 0) + { + /* Check if we stay inside the bounds on the right side */ + if (prgn->rdh.rcBound.bottom > (MAX_COORD - cy)) + { + return FALSE; + } + } + else + { + /* Check if we stay inside the bounds on the left side */ + if (prgn->rdh.rcBound.top < (MIN_COORD - cy)) + { + return FALSE; + } + } + + /* Loop to move the rects */ + prcl = prgn->Buffer; + for (i = 0; i < prgn->rdh.nCount; i++) + { + prcl[i].left += cx; + prcl[i].right += cx; + prcl[i].top += cy; + prcl[i].bottom += cy; + } + + /* Finally update the bounds rect */ + if (prgn->Buffer != &prgn->rdh.rcBound) + { + prgn->rdh.rcBound.left += cx; + prgn->rdh.rcBound.right += cx; + prgn->rdh.rcBound.top += cy; + prgn->rdh.rcBound.bottom += cy; + } + + return TRUE; } /*********************************************************************** @@ -3786,7 +3839,14 @@ NtGdiOffsetRgn( return ERROR; } - ret = REGION_iOffsetRgn(rgn, XOffset, YOffset); + if (!REGION_bOffsetRgn(rgn, XOffset, YOffset)) + { + ret = ERROR; + } + else + { + ret = REGION_Complexity(rgn); + } RGNOBJAPI_Unlock(rgn); return ret; diff --git a/reactos/win32ss/gdi/ntgdi/region.h b/reactos/win32ss/gdi/ntgdi/region.h index 51fa032fc80..101fe7263d9 100644 --- a/reactos/win32ss/gdi/ntgdi/region.h +++ b/reactos/win32ss/gdi/ntgdi/region.h @@ -62,7 +62,13 @@ GreCreatePolyPolygonRgn( _In_ ULONG cPolygons, _In_ INT iMode); -INT FASTCALL REGION_iOffsetRgn(PREGION,INT,INT); +BOOL +FASTCALL +REGION_bOffsetRgn( + _Inout_ PREGION prgn, + _In_ INT cx, + _In_ INT cy); + BOOL FASTCALL IntRectInRegion(HRGN,LPRECTL); INT FASTCALL IntGdiCombineRgn(PREGION, PREGION, PREGION, INT); diff --git a/reactos/win32ss/user/ntuser/painting.c b/reactos/win32ss/user/ntuser/painting.c index 9fcc32e8103..30cee5a6bf9 100644 --- a/reactos/win32ss/user/ntuser/painting.c +++ b/reactos/win32ss/user/ntuser/painting.c @@ -460,11 +460,11 @@ IntInvalidateWindows(PWND Wnd, PREGION Rgn, ULONG Flags) PREGION RgnClip = RGNOBJAPI_Lock(Wnd->hrgnClip, NULL); if (RgnClip) { - REGION_iOffsetRgn(Rgn, + REGION_bOffsetRgn(Rgn, -Wnd->rcWindow.left, -Wnd->rcWindow.top); RgnType = IntGdiCombineRgn(Rgn, Rgn, RgnClip, RGN_AND); - REGION_iOffsetRgn(Rgn, + REGION_bOffsetRgn(Rgn, Wnd->rcWindow.left, Wnd->rcWindow.top); RGNOBJAPI_Unlock(RgnClip); @@ -682,7 +682,7 @@ co_UserRedrawWindow( } else { - REGION_iOffsetRgn(TmpRgn, Window->rcClient.left, Window->rcClient.top); + REGION_bOffsetRgn(TmpRgn, Window->rcClient.left, Window->rcClient.top); } } else if (UpdateRect != NULL) @@ -690,7 +690,7 @@ co_UserRedrawWindow( if (!RECTL_bIsEmptyRect(UpdateRect)) { TmpRgn = IntSysCreateRectpRgnIndirect(UpdateRect); - REGION_iOffsetRgn(TmpRgn, Window->rcClient.left, Window->rcClient.top); + REGION_bOffsetRgn(TmpRgn, Window->rcClient.left, Window->rcClient.top); } } else if ((Flags & (RDW_INVALIDATE | RDW_FRAME)) == (RDW_INVALIDATE | RDW_FRAME) || @@ -1229,7 +1229,7 @@ co_UserGetUpdateRgn(PWND Window, PREGION Rgn, BOOL bErase) IntIntersectWithParents(Window, &Rect); REGION_SetRectRgn(Rgn, Rect.left, Rect.top, Rect.right, Rect.bottom); RegionType = IntGdiCombineRgn(Rgn, Rgn, UpdateRgn, RGN_AND); - REGION_iOffsetRgn(Rgn, -Window->rcClient.left, -Window->rcClient.top); + REGION_bOffsetRgn(Rgn, -Window->rcClient.left, -Window->rcClient.top); RGNOBJAPI_Unlock(UpdateRgn); if (bErase && RegionType != NULLREGION && RegionType != ERROR) @@ -1565,7 +1565,7 @@ UserScrollDC( /* Substract the part of the dest that was visible in source */ IntGdiCombineRgn(RgnTmp, RgnTmp, pDC->prgnVis, RGN_AND); - REGION_iOffsetRgn(RgnTmp, dx, dy); + REGION_bOffsetRgn(RgnTmp, dx, dy); Result = IntGdiCombineRgn(RgnOwn, RgnOwn, RgnTmp, RGN_DIFF); /* DO NOT Unlock DC while messing with prgnVis! */ @@ -1851,7 +1851,7 @@ NtUserScrollWindowEx( RgnWinupd = IntSysCreateRectpRgn( 0, 0, 0, 0); IntGdiCombineRgn( RgnWinupd, RgnTemp, 0, RGN_COPY); } - REGION_iOffsetRgn(RgnTemp, dx, dy); + REGION_bOffsetRgn(RgnTemp, dx, dy); IntGdiCombineRgn(RgnTemp, RgnTemp, RgnClip, RGN_AND); if (hrgnUpdate) IntGdiCombineRgn( RgnWinupd, RgnWinupd, RgnTemp, RGN_OR ); diff --git a/reactos/win32ss/user/ntuser/vis.c b/reactos/win32ss/user/ntuser/vis.c index 2c5dd9a571f..55bbb944996 100644 --- a/reactos/win32ss/user/ntuser/vis.c +++ b/reactos/win32ss/user/ntuser/vis.c @@ -81,9 +81,9 @@ VIS_ComputeVisibleRegion( PREGION SiblingClipRgn = RGNOBJAPI_Lock(CurrentSibling->hrgnClip, NULL); if (SiblingClipRgn) { - REGION_iOffsetRgn(ClipRgn, -CurrentSibling->rcWindow.left, -CurrentSibling->rcWindow.top); + REGION_bOffsetRgn(ClipRgn, -CurrentSibling->rcWindow.left, -CurrentSibling->rcWindow.top); IntGdiCombineRgn(ClipRgn, ClipRgn, SiblingClipRgn, RGN_AND); - REGION_iOffsetRgn(ClipRgn, CurrentSibling->rcWindow.left, CurrentSibling->rcWindow.top); + REGION_bOffsetRgn(ClipRgn, CurrentSibling->rcWindow.left, CurrentSibling->rcWindow.top); RGNOBJAPI_Unlock(SiblingClipRgn); } } @@ -113,9 +113,9 @@ VIS_ComputeVisibleRegion( PREGION CurrentRgnClip = RGNOBJAPI_Lock(CurrentWindow->hrgnClip, NULL); if (CurrentRgnClip) { - REGION_iOffsetRgn(ClipRgn, -CurrentWindow->rcWindow.left, -CurrentWindow->rcWindow.top); + REGION_bOffsetRgn(ClipRgn, -CurrentWindow->rcWindow.left, -CurrentWindow->rcWindow.top); IntGdiCombineRgn(ClipRgn, ClipRgn, CurrentRgnClip, RGN_AND); - REGION_iOffsetRgn(ClipRgn, CurrentWindow->rcWindow.left, CurrentWindow->rcWindow.top); + REGION_bOffsetRgn(ClipRgn, CurrentWindow->rcWindow.left, CurrentWindow->rcWindow.top); RGNOBJAPI_Unlock(CurrentRgnClip); } } @@ -131,9 +131,9 @@ VIS_ComputeVisibleRegion( PREGION WndRgnClip = RGNOBJAPI_Lock(Wnd->hrgnClip, NULL); if (WndRgnClip) { - REGION_iOffsetRgn(VisRgn, -Wnd->rcWindow.left, -Wnd->rcWindow.top); + REGION_bOffsetRgn(VisRgn, -Wnd->rcWindow.left, -Wnd->rcWindow.top); IntGdiCombineRgn(VisRgn, VisRgn, WndRgnClip, RGN_AND); - REGION_iOffsetRgn(VisRgn, Wnd->rcWindow.left, Wnd->rcWindow.top); + REGION_bOffsetRgn(VisRgn, Wnd->rcWindow.left, Wnd->rcWindow.top); RGNOBJAPI_Unlock(WndRgnClip); } } @@ -160,7 +160,7 @@ co_VIS_WindowLayoutChanged( return; IntGdiCombineRgn(TempRgn, NewlyExposed, NULL, RGN_COPY); - REGION_iOffsetRgn(TempRgn, + REGION_bOffsetRgn(TempRgn, Wnd->rcWindow.left - Parent->rcClient.left, Wnd->rcWindow.top - Parent->rcClient.top); diff --git a/reactos/win32ss/user/ntuser/windc.c b/reactos/win32ss/user/ntuser/windc.c index 9fc083aa6b1..4935189e51e 100644 --- a/reactos/win32ss/user/ntuser/windc.c +++ b/reactos/win32ss/user/ntuser/windc.c @@ -873,7 +873,7 @@ DceResetActiveDCEs(PWND Window) if (NULL != dc->dclevel.prgnClip) { - REGION_iOffsetRgn(dc->dclevel.prgnClip, DeltaX, DeltaY); + REGION_bOffsetRgn(dc->dclevel.prgnClip, DeltaX, DeltaY); dc->fs |= DC_FLAG_DIRTY_RAO; } if (NULL != pDCE->hrgnClip) diff --git a/reactos/win32ss/user/ntuser/winpos.c b/reactos/win32ss/user/ntuser/winpos.c index a80347e7296..3e780417a30 100644 --- a/reactos/win32ss/user/ntuser/winpos.c +++ b/reactos/win32ss/user/ntuser/winpos.c @@ -1762,7 +1762,7 @@ co_WinPosSetWindowPos( } else if(VisBefore) { - REGION_iOffsetRgn(VisBefore, -Window->rcWindow.left, -Window->rcWindow.top); + REGION_bOffsetRgn(VisBefore, -Window->rcWindow.left, -Window->rcWindow.top); } /* Calculate the non client area for resizes, as this is used in the copy region */ @@ -1779,7 +1779,7 @@ co_WinPosSetWindowPos( } else if(VisBeforeJustClient) { - REGION_iOffsetRgn(VisBeforeJustClient, -Window->rcWindow.left, -Window->rcWindow.top); + REGION_bOffsetRgn(VisBeforeJustClient, -Window->rcWindow.left, -Window->rcWindow.top); } } } @@ -1864,7 +1864,7 @@ co_WinPosSetWindowPos( } else if(VisAfter) { - REGION_iOffsetRgn(VisAfter, -Window->rcWindow.left, -Window->rcWindow.top); + REGION_bOffsetRgn(VisAfter, -Window->rcWindow.left, -Window->rcWindow.top); } /* @@ -1905,9 +1905,9 @@ co_WinPosSetWindowPos( PREGION RgnUpdate = RGNOBJAPI_Lock(Window->hrgnUpdate, NULL); if (RgnUpdate) { - REGION_iOffsetRgn(CopyRgn, NewWindowRect.left, NewWindowRect.top); + REGION_bOffsetRgn(CopyRgn, NewWindowRect.left, NewWindowRect.top); IntGdiCombineRgn(CopyRgn, CopyRgn, RgnUpdate, RGN_DIFF); - REGION_iOffsetRgn(CopyRgn, -NewWindowRect.left, -NewWindowRect.top); + REGION_bOffsetRgn(CopyRgn, -NewWindowRect.left, -NewWindowRect.top); RGNOBJAPI_Unlock(RgnUpdate); } } @@ -1939,7 +1939,7 @@ co_WinPosSetWindowPos( * to create a copy of CopyRgn and pass that. We need CopyRgn later */ IntGdiCombineRgn(DcRgnObj, CopyRgn, NULL, RGN_COPY); - REGION_iOffsetRgn(DcRgnObj, NewWindowRect.left, NewWindowRect.top); + REGION_bOffsetRgn(DcRgnObj, NewWindowRect.left, NewWindowRect.top); RGNOBJAPI_Unlock(DcRgnObj); Dc = UserGetDCEx( Window, DcRgn, @@ -1992,7 +1992,7 @@ co_WinPosSetWindowPos( PWND Parent = Window->spwndParent; - REGION_iOffsetRgn( DirtyRgn, + REGION_bOffsetRgn( DirtyRgn, Window->rcWindow.left, Window->rcWindow.top); if ( (Window->style & WS_CHILD) && @@ -2027,7 +2027,7 @@ co_WinPosSetWindowPos( if (ExposedRgn) { RgnType = IntGdiCombineRgn(ExposedRgn, VisBefore, NULL, RGN_COPY); - REGION_iOffsetRgn(ExposedRgn, + REGION_bOffsetRgn(ExposedRgn, OldWindowRect.left - NewWindowRect.left, OldWindowRect.top - NewWindowRect.top);