[dchapyshev] 72636: [WIN32KNT_APITEST] - Fix copypasta [NTUSER] - User's buffer has to be checked before check of size parameters, differently we cannot safely check the sizes of structures
Author: dchapyshev Date: Fri Sep 9 20:44:34 2016 New Revision: 72636
URL: http://svn.reactos.org/svn/reactos?rev=72636&view=rev Log: [WIN32KNT_APITEST] - Fix copypasta
[NTUSER] - User's buffer has to be checked before check of size parameters, differently we cannot safely check the sizes of structures
Modified: trunk/reactos/win32ss/user/ntuser/sysparams.c trunk/rostests/apitests/win32nt/ntuser/NtUserSystemParametersInfo.c
Modified: trunk/reactos/win32ss/user/ntuser/sysparams.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/syspara... ============================================================================== --- trunk/reactos/win32ss/user/ntuser/sysparams.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/ntuser/sysparams.c [iso-8859-1] Fri Sep 9 20:44:34 2016 @@ -222,7 +222,7 @@ TRACE("Enter SpiUpdatePerUserSystemParameters\n");
/* Clear the structure */ - memset(&gspv, 0, sizeof(gspv)); + RtlZeroMemory(&gspv, sizeof(gspv));
/* Load mouse settings */ gspv.caiMouse.FirstThreshold = SpiLoadMouse(VAL_MOUSE1, 6); @@ -424,7 +424,7 @@ // FIXME: get rid of the flags and only use this from um. kernel can access data directly. static UINT_PTR -SpiMemCopy(PVOID pvDst, PVOID pvSrc, ULONG cbSize, BOOL bProtect, BOOL bToUser) +SpiMemCopy(PVOID pvDst, PVOID pvSrc, ULONG cbSize, BOOL bProtect) { NTSTATUS Status = STATUS_SUCCESS;
@@ -432,15 +432,7 @@ { _SEH2_TRY { - if (bToUser) - { - ProbeForWrite(pvDst, cbSize, 1); - } - else - { - ProbeForRead(pvSrc, cbSize, 1); - } - memcpy(pvDst, pvSrc, cbSize); + RtlCopyMemory(pvDst, pvSrc, cbSize); } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { @@ -450,14 +442,15 @@ } else { - memcpy(pvDst, pvSrc, cbSize); + RtlCopyMemory(pvDst, pvSrc, cbSize); }
if (!NT_SUCCESS(Status)) { SetLastNtError(Status); - ERR("SpiMemCopy failed, pvDst=%p, pvSrc=%p, bProtect=%d, bToUser=%d\n", pvDst, pvSrc, bProtect, bToUser); - } + ERR("SpiMemCopy failed, pvDst=%p, pvSrc=%p, bProtect=%d\n", pvDst, pvSrc, bProtect); + } + return NT_SUCCESS(Status); }
@@ -466,7 +459,7 @@ SpiGet(PVOID pvParam, PVOID pvData, ULONG cbSize, FLONG fl) { REQ_INTERACTIVE_WINSTA(ERROR_ACCESS_DENIED); - return SpiMemCopy(pvParam, pvData, cbSize, fl & SPIF_PROTECT, TRUE); + return SpiMemCopy(pvParam, pvData, cbSize, fl & SPIF_PROTECT); }
static inline @@ -474,7 +467,7 @@ SpiSet(PVOID pvData, PVOID pvParam, ULONG cbSize, FLONG fl) { REQ_INTERACTIVE_WINSTA(ERROR_REQUIRES_INTERACTIVE_WINDOWSTATION); - return SpiMemCopy(pvData, pvParam, cbSize, fl & SPIF_PROTECT, FALSE); + return SpiMemCopy(pvData, pvParam, cbSize, fl & SPIF_PROTECT); }
static inline @@ -631,13 +624,13 @@ }
/* Capture UNICODE_STRING */ - bResult = SpiMemCopy(&ustr, pvParam, sizeof(ustr), fl & SPIF_PROTECT, 0); + bResult = SpiMemCopy(&ustr, pvParam, sizeof(ustr), fl & SPIF_PROTECT); if (!bResult) return 0; if (ustr.Length > MAX_PATH * sizeof(WCHAR)) return 0;
/* Copy the string buffer name */ - bResult = SpiMemCopy(gspv.awcWallpaper, ustr.Buffer, ustr.Length, fl & SPIF_PROTECT, 0); + bResult = SpiMemCopy(gspv.awcWallpaper, ustr.Buffer, ustr.Length, fl & SPIF_PROTECT); if (!bResult) return 0;
/* Update the UNICODE_STRING */ @@ -911,11 +904,31 @@ return SpiSetInt(&gspv.bDragFullWindows, uiParam, KEY_DESKTOP, VAL_DRAG, fl);
case SPI_GETNONCLIENTMETRICS: + { + LPNONCLIENTMETRICSW metrics = (LPNONCLIENTMETRICSW)pvParam; + + if (uiParam != 0 && uiParam != sizeof(NONCLIENTMETRICSW)) + return 0; + + if (!metrics || metrics->cbSize != sizeof(NONCLIENTMETRICSW)) + return 0; + return SpiGet(pvParam, &gspv.ncm, sizeof(NONCLIENTMETRICSW), fl); + }
case SPI_SETNONCLIENTMETRICS: + { + LPNONCLIENTMETRICSW metrics = (LPNONCLIENTMETRICSW)pvParam; + + if (uiParam != 0 && uiParam != sizeof(NONCLIENTMETRICSW)) + return 0; + + if (!metrics || metrics->cbSize != sizeof(NONCLIENTMETRICSW)) + return 0; + if (!SpiSet(&gspv.ncm, pvParam, sizeof(NONCLIENTMETRICSW), fl)) return 0; + if (fl & SPIF_UPDATEINIFILE) { SpiStoreMetric(VAL_BORDER, gspv.ncm.iBorderWidth); @@ -936,20 +949,44 @@ SpiStoreFont(L"StatusFont", &gspv.ncm.lfStatusFont); SpiStoreFont(L"MessageFont", &gspv.ncm.lfMessageFont); } + if(!SpiNotifyNCMetricsChanged()) return 0; + return (UINT_PTR)KEY_METRIC; + }
case SPI_GETMINIMIZEDMETRICS: + { + LPMINIMIZEDMETRICS metrics = (LPMINIMIZEDMETRICS)pvParam; + + if (uiParam != 0 && uiParam != sizeof(MINIMIZEDMETRICS)) + return 0; + + if (!metrics || metrics->cbSize != sizeof(MINIMIZEDMETRICS)) + return 0; + return SpiGet(pvParam, &gspv.mm, sizeof(MINIMIZEDMETRICS), fl); + }
case SPI_SETMINIMIZEDMETRICS: + { + LPMINIMIZEDMETRICS metrics = (LPMINIMIZEDMETRICS)pvParam; + + if (uiParam != 0 && uiParam != sizeof(MINIMIZEDMETRICS)) + return 0; + + if (!metrics || metrics->cbSize != sizeof(MINIMIZEDMETRICS)) + return 0; + if (!SpiSet(&gspv.mm, pvParam, sizeof(MINIMIZEDMETRICS), fl)) return 0; + gspv.mm.iWidth = max(0, gspv.mm.iWidth); gspv.mm.iHorzGap = max(0, gspv.mm.iHorzGap); gspv.mm.iVertGap = max(0, gspv.mm.iVertGap); gspv.mm.iArrange = gspv.mm.iArrange & 0xf; + if (fl & SPIF_UPDATEINIFILE) { SpiStoreMetric(L"MinWidth", gspv.mm.iWidth); @@ -957,14 +994,36 @@ SpiStoreMetric(L"MinVertGap", gspv.mm.iVertGap); SpiStoreMetric(L"MinArrange", gspv.mm.iArrange); } + return (UINT_PTR)KEY_METRIC; + }
case SPI_GETICONMETRICS: - return SpiGet(pvParam, &gspv.im, sizeof(ICONMETRICS), fl); + { + LPICONMETRICSW IconMetrics = (LPICONMETRICSW)pvParam; + + if (uiParam != 0 && uiParam != sizeof(ICONMETRICSW)) + return 0; + + if (!IconMetrics || IconMetrics->cbSize != sizeof(ICONMETRICSW)) + return 0; + + return SpiGet(pvParam, &gspv.im, sizeof(ICONMETRICSW), fl); + }
case SPI_SETICONMETRICS: + { + LPICONMETRICS IconMetrics = (LPICONMETRICS)pvParam; + + if (uiParam != 0 && uiParam != sizeof(ICONMETRICS)) + return 0; + + if (!IconMetrics || IconMetrics->cbSize != sizeof(ICONMETRICS)) + return 0; + if (!SpiSet(&gspv.im, pvParam, sizeof(ICONMETRICS), fl)) return 0; + if (fl & SPIF_UPDATEINIFILE) { SpiStoreMetric(VAL_ICONSPC, gspv.im.iHorzSpacing); @@ -973,6 +1032,7 @@ SpiStoreFont(L"IconFont", &gspv.im.lfFont); } return (UINT_PTR)KEY_METRIC; + }
case SPI_GETWORKAREA: { @@ -1713,6 +1773,276 @@ return 0; }
+static BOOL +SpiGetSetProbeBuffer(UINT uiAction, UINT uiParam, PVOID pvParam) +{ + BOOL bToUser = TRUE; + ULONG cbSize = 0; + + switch (uiAction) + { + case SPI_GETBEEP: + case SPI_GETBORDER: + case SPI_GETKEYBOARDSPEED: + case SPI_GETSCREENSAVETIMEOUT: + case SPI_GETSCREENSAVEACTIVE: + case SPI_GETGRIDGRANULARITY: + case SPI_GETKEYBOARDDELAY: + case SPI_GETICONTITLEWRAP: + case SPI_GETMENUDROPALIGNMENT: + case SPI_GETFASTTASKSWITCH: + case SPI_GETDRAGFULLWINDOWS: + case SPI_GETSHOWSOUNDS: + case SPI_GETKEYBOARDPREF: + case SPI_GETSCREENREADER: + case SPI_GETFONTSMOOTHING: + case SPI_GETLOWPOWERTIMEOUT: + case SPI_GETPOWEROFFTIMEOUT: + case SPI_GETLOWPOWERACTIVE: + case SPI_GETPOWEROFFACTIVE: + case SPI_GETMOUSETRAILS: + case SPI_GETSNAPTODEFBUTTON: + case SPI_GETMOUSEHOVERWIDTH: + case SPI_GETMOUSEHOVERHEIGHT: + case SPI_GETMOUSEHOVERTIME: + case SPI_GETWHEELSCROLLLINES: + case SPI_GETMENUSHOWDELAY: +#if (_WIN32_WINNT >= 0x0600) + case SPI_GETWHEELSCROLLCHARS: +#endif + case SPI_GETSHOWIMEUI: + case SPI_GETMOUSESPEED: + case SPI_GETSCREENSAVERRUNNING: +#if(WINVER >= 0x0600) + case SPI_GETSCREENSAVESECURE: +#endif + case SPI_GETACTIVEWINDOWTRACKING: + case SPI_GETMENUANIMATION: + case SPI_GETCOMBOBOXANIMATION: + case SPI_GETLISTBOXSMOOTHSCROLLING: + case SPI_GETGRADIENTCAPTIONS: + case SPI_GETKEYBOARDCUES: + case SPI_GETACTIVEWNDTRKZORDER: + case SPI_GETHOTTRACKING: + case SPI_GETMENUFADE: + case SPI_GETSELECTIONFADE: + case SPI_GETTOOLTIPANIMATION: + case SPI_GETTOOLTIPFADE: + case SPI_GETCURSORSHADOW: + case SPI_GETUIEFFECTS: + case SPI_GETMOUSESONAR: + case SPI_GETMOUSECLICKLOCK: + case SPI_GETMOUSEVANISH: + case SPI_GETFLATMENU: + case SPI_GETDROPSHADOW: + case SPI_GETBLOCKSENDINPUTRESETS: +#if(_WIN32_WINNT >= 0x0600) + case SPI_GETDISABLEOVERLAPPEDCONTENT: + case SPI_GETCLIENTAREAANIMATION: + case SPI_GETCLEARTYPE: + case SPI_GETSPEECHRECOGNITION: +#endif + case SPI_GETFOREGROUNDLOCKTIMEOUT: + case SPI_GETACTIVEWNDTRKTIMEOUT: + case SPI_GETFOREGROUNDFLASHCOUNT: + case SPI_GETCARETWIDTH: + case SPI_GETMOUSECLICKLOCKTIME: + case SPI_GETFONTSMOOTHINGTYPE: + case SPI_GETFONTSMOOTHINGCONTRAST: + case SPI_GETFOCUSBORDERWIDTH: + case SPI_GETFOCUSBORDERHEIGHT: + case SPI_GETFONTSMOOTHINGORIENTATION: + cbSize = sizeof(INT); + break; + + case SPI_ICONHORIZONTALSPACING: + case SPI_ICONVERTICALSPACING: + if (pvParam) cbSize = sizeof(INT); + break; + + case SPI_GETMOUSE: + cbSize = 3 * sizeof(INT); + break; + + case SPI_GETDESKWALLPAPER: + cbSize = min(uiParam, gspv.ustrWallpaper.Length + 1UL); + break; + + case SPI_GETICONTITLELOGFONT: + cbSize = sizeof(LOGFONTW); + break; + + case SPI_GETNONCLIENTMETRICS: + cbSize = sizeof(NONCLIENTMETRICSW); + break; + + case SPI_GETMINIMIZEDMETRICS: + cbSize = sizeof(MINIMIZEDMETRICS); + break; + + case SPI_GETICONMETRICS: + cbSize = sizeof(ICONMETRICS); + break; + + case SPI_GETWORKAREA: + cbSize = sizeof(RECTL); + break; + + case SPI_GETFILTERKEYS: + cbSize = sizeof(FILTERKEYS); + break; + + case SPI_GETTOGGLEKEYS: + cbSize = sizeof(TOGGLEKEYS); + break; + + case SPI_GETMOUSEKEYS: + cbSize = sizeof(MOUSEKEYS); + break; + + case SPI_GETSTICKYKEYS: + cbSize = sizeof(STICKYKEYS); + break; + + case SPI_GETACCESSTIMEOUT: + cbSize = sizeof(ACCESSTIMEOUT); + break; + + case SPI_GETSERIALKEYS: + cbSize = sizeof(SERIALKEYS); + break; + + case SPI_GETSOUNDSENTRY: + cbSize = sizeof(SOUNDSENTRY); + break; + + case SPI_GETHIGHCONTRAST: + cbSize = sizeof(HIGHCONTRAST); + break; + + case SPI_GETANIMATION: + cbSize = sizeof(ANIMATIONINFO); + break; + + case SPI_GETDEFAULTINPUTLANG: + cbSize = sizeof(HKL); + break; + +#if(WINVER >= 0x0600) + case SPI_GETAUDIODESCRIPTION: + cbSize = sizeof(AUDIODESCRIPTION); + break; +#endif + + case SPI_SETMOUSE: + cbSize = 3 * sizeof(INT); + bToUser = FALSE; + break; + + case SPI_SETICONTITLELOGFONT: + cbSize = sizeof(LOGFONTW); + bToUser = FALSE; + break; + + case SPI_SETNONCLIENTMETRICS: + cbSize = sizeof(NONCLIENTMETRICSW); + bToUser = FALSE; + break; + + case SPI_SETMINIMIZEDMETRICS: + cbSize = sizeof(MINIMIZEDMETRICS); + bToUser = FALSE; + break; + + case SPI_SETICONMETRICS: + cbSize = sizeof(ICONMETRICS); + bToUser = FALSE; + break; + + case SPI_SETWORKAREA: + cbSize = sizeof(RECTL); + bToUser = FALSE; + break; + + case SPI_SETFILTERKEYS: + cbSize = sizeof(FILTERKEYS); + bToUser = FALSE; + break; + + case SPI_SETTOGGLEKEYS: + cbSize = sizeof(TOGGLEKEYS); + bToUser = FALSE; + break; + + case SPI_SETMOUSEKEYS: + cbSize = sizeof(MOUSEKEYS); + bToUser = FALSE; + break; + + case SPI_SETSTICKYKEYS: + cbSize = sizeof(STICKYKEYS); + bToUser = FALSE; + break; + + case SPI_SETACCESSTIMEOUT: + cbSize = sizeof(ACCESSTIMEOUT); + bToUser = FALSE; + break; + + case SPI_SETSERIALKEYS: + cbSize = sizeof(SERIALKEYS); + bToUser = FALSE; + break; + + case SPI_SETSOUNDSENTRY: + cbSize = sizeof(SOUNDSENTRY); + bToUser = FALSE; + break; + + case SPI_SETHIGHCONTRAST: + cbSize = sizeof(HIGHCONTRAST); + bToUser = FALSE; + break; + + case SPI_SETANIMATION: + cbSize = sizeof(ANIMATIONINFO); + bToUser = FALSE; + break; + + case SPI_SETDEFAULTINPUTLANG: + cbSize = sizeof(HKL); + bToUser = FALSE; + break; + + case SPI_SETMOUSESPEED: + cbSize = sizeof(INT); + bToUser = FALSE; + break; + } + + if (cbSize) + { + _SEH2_TRY + { + if (bToUser) + { + ProbeForWrite(pvParam, cbSize, sizeof(UCHAR)); + } + else + { + ProbeForRead(pvParam, cbSize, sizeof(UCHAR)); + } + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + _SEH2_YIELD(return FALSE); + } + _SEH2_END; + } + + return TRUE; +} + BOOL FASTCALL UserSystemParametersInfo( @@ -1739,6 +2069,12 @@ ERR("UserSystemParametersInfo called without active window station.\n"); //ASSERT(FALSE); //return FALSE; + } + + if ((fWinIni & SPIF_PROTECT) && !SpiGetSetProbeBuffer(uiAction, uiParam, pvParam)) + { + EngSetLastError(ERROR_NOACCESS); + return FALSE; }
/* Do the actual operation */
Modified: trunk/rostests/apitests/win32nt/ntuser/NtUserSystemParametersInfo.c URL: http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/win32nt/ntuser/Nt... ============================================================================== --- trunk/rostests/apitests/win32nt/ntuser/NtUserSystemParametersInfo.c [iso-8859-1] (original) +++ trunk/rostests/apitests/win32nt/ntuser/NtUserSystemParametersInfo.c [iso-8859-1] Fri Sep 9 20:44:34 2016 @@ -742,9 +742,9 @@ NONCLIENTMETRICSW metrics;
metrics.cbSize = sizeof(NONCLIENTMETRICSW); - TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(NONCLIENTMETRICSW), &metrics, 0) == 1); - TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(NONCLIENTMETRICSW) + 1, &metrics, 0) == 0); - TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(NONCLIENTMETRICSW), (PVOID)0xdeadbeef, 0) == 0); + TEST(NtUserSystemParametersInfo(SPI_GETNONCLIENTMETRICS, sizeof(NONCLIENTMETRICSW), &metrics, 0) == 1); + TEST(NtUserSystemParametersInfo(SPI_GETNONCLIENTMETRICS, sizeof(NONCLIENTMETRICSW) + 1, &metrics, 0) == 0); + TEST(NtUserSystemParametersInfo(SPI_GETNONCLIENTMETRICS, sizeof(NONCLIENTMETRICSW), (PVOID)0xdeadbeef, 0) == 0); }
void @@ -753,20 +753,20 @@ MINIMIZEDMETRICS metrics;
metrics.cbSize = sizeof(MINIMIZEDMETRICS); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(MINIMIZEDMETRICS), (PVOID)&metrics, 0) == 1); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(MINIMIZEDMETRICS) + 1, (PVOID)&metrics, 0) == 0); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(MINIMIZEDMETRICS), (PVOID)0xdeadbeef, 0) == 0); + TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(MINIMIZEDMETRICS), (PVOID)&metrics, 0) == 1); + TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(MINIMIZEDMETRICS) + 1, (PVOID)&metrics, 0) == 0); + TEST(NtUserSystemParametersInfo(SPI_GETMINIMIZEDMETRICS, sizeof(MINIMIZEDMETRICS), (PVOID)0xdeadbeef, 0) == 0); }
void Test_SPI_SETICONMETRICS(void) { - ICONMETRICS metrics; - - metrics.cbSize = sizeof(ICONMETRICS); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICS), (PVOID)&metrics, 0) == 1); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICS) + 1, (PVOID)&metrics, 0) == 0); - TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICS), (PVOID)0xdeadbeef, 0) == 0); + ICONMETRICSW metrics; + + metrics.cbSize = sizeof(ICONMETRICSW); + TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICSW), (PVOID)&metrics, 0) == 1); + TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICSW) + 1, (PVOID)&metrics, 0) == 0); + TEST(NtUserSystemParametersInfo(SPI_GETICONMETRICS, sizeof(ICONMETRICSW), (PVOID)0xdeadbeef, 0) == 0); }
void
-
dchapyshev＠svn.reactos.org