[PATCH v6 0/2] MR2262: winspool: Check dmSize in IsValidDevmodeW
When dmSize is zero or greter than size of input buffer, `IsValidDevmodeW()` failed in Windows 10. But current implementation in wine, it will return `true` because there is no check to `dm.dmSize`. -- v6: winspool: Check the alignment of dmSize in IsValidDevmodeW() https://gitlab.winehq.org/wine/wine/-/merge_requests/2262
From: Tingzhong Luo <luotingzhong(a)uniontech.com> --- dlls/winspool.drv/info.c | 5 ++++- dlls/winspool.drv/tests/info.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/dlls/winspool.drv/info.c b/dlls/winspool.drv/info.c index 54ecb81d3be..8a85dc97c73 100644 --- a/dlls/winspool.drv/info.c +++ b/dlls/winspool.drv/info.c @@ -1927,9 +1927,12 @@ BOOL WINAPI IsValidDevmodeW(PDEVMODEW dm, SIZE_T size) #undef F_SIZE }; int i; + const DWORD offFields = FIELD_OFFSET(DEVMODEW, dmFields) + sizeof(dm->dmFields); if (!dm) return FALSE; - if (size < FIELD_OFFSET(DEVMODEW, dmFields) + sizeof(dm->dmFields)) return FALSE; + if (size < offFields) return FALSE; + if (dm->dmSize < offFields || size < dm->dmSize + dm->dmDriverExtra) return FALSE; + if (((dm->dmSize - offFields) % 4) && dm->dmDriverExtra) return FALSE; for (i = 0; i < ARRAY_SIZE(map); i++) if ((dm->dmFields & map[i].flag) && size < map[i].size) diff --git a/dlls/winspool.drv/tests/info.c b/dlls/winspool.drv/tests/info.c index 92b042e354a..e8beceeb571 100644 --- a/dlls/winspool.drv/tests/info.c +++ b/dlls/winspool.drv/tests/info.c @@ -3046,6 +3046,30 @@ static void test_IsValidDevmodeW(void) { DM_NUP, FIELD_OFFSET(DEVMODEW, u2.dmNup) + 4, TRUE }, }; + static const struct + { + DWORD dmSize; + DWORD dmDriverExtra; + DWORD bufSize; + BOOL ret; + } size_test[] = + { + { FIELD_OFFSET(DEVMODEW, dmFields) + 3, 1, FIELD_OFFSET(DEVMODEW, dmFields) + 4, FALSE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 4, 1, FIELD_OFFSET(DEVMODEW, dmFields) + 8, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 4, 2, FIELD_OFFSET(DEVMODEW, dmFields) + 8, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 4, 3, FIELD_OFFSET(DEVMODEW, dmFields) + 8, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 4, 4, FIELD_OFFSET(DEVMODEW, dmFields) + 8, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 4, 5, FIELD_OFFSET(DEVMODEW, dmFields) + 8, FALSE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 5, 0, FIELD_OFFSET(DEVMODEW, dmFields) + 8, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 5, 1, FIELD_OFFSET(DEVMODEW, dmFields) + 8, FALSE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 12, 1, FIELD_OFFSET(DEVMODEW, dmFields) + 16, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 12, 2, FIELD_OFFSET(DEVMODEW, dmFields) + 16, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 12, 3, FIELD_OFFSET(DEVMODEW, dmFields) + 16, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 12, 4, FIELD_OFFSET(DEVMODEW, dmFields) + 16, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 12, 5, FIELD_OFFSET(DEVMODEW, dmFields) + 16, FALSE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 13, 0, FIELD_OFFSET(DEVMODEW, dmFields) + 16, TRUE }, + { FIELD_OFFSET(DEVMODEW, dmFields) + 13, 1, FIELD_OFFSET(DEVMODEW, dmFields) + 16, FALSE }, + }; DEVMODEW dm; int i; BOOL ret; @@ -3065,6 +3089,15 @@ static void test_IsValidDevmodeW(void) ret = IsValidDevmodeW(&dm, dm.dmSize); ok(ret == test[i].ret, "%d: got %d\n", i, ret); } + + dm.dmFields = 0; + for (i = 0; i < ARRAY_SIZE(size_test); i++) + { + dm.dmSize = size_test[i].dmSize; + dm.dmDriverExtra = size_test[i].dmDriverExtra; + ret = IsValidDevmodeW(&dm, size_test[i].bufSize); + ok(ret == size_test[i].ret, "%d: got %d\n", i, ret); + } } START_TEST(info) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2262
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/winspool.drv/info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winspool.drv/info.c b/dlls/winspool.drv/info.c index 8a85dc97c73..86b37d7743e 100644 --- a/dlls/winspool.drv/info.c +++ b/dlls/winspool.drv/info.c @@ -1935,7 +1935,7 @@ BOOL WINAPI IsValidDevmodeW(PDEVMODEW dm, SIZE_T size) if (((dm->dmSize - offFields) % 4) && dm->dmDriverExtra) return FALSE; for (i = 0; i < ARRAY_SIZE(map); i++) - if ((dm->dmFields & map[i].flag) && size < map[i].size) + if ((dm->dmFields & map[i].flag) && dm->dmSize < map[i].size) return FALSE; return TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2262
participants (3)
-
Piotr Caban -
Tingzhong Luo -
Tingzhong Luo (@tzluo)