[PATCH 0/1] MR2262: winspool: Check dmSize in IsValidDevmodeW

[Tingzhong Luo (＠tzluo)]

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`.

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

[PATCH 1/1] winspool: Check dmSize in IsValidDevmodeW

[Tingzhong Luo]

From: Tingzhong Luo <luotingzhong(a)uniontech.com>

---
 dlls/winspool.drv/info.c       | 1 +
 dlls/winspool.drv/tests/info.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/dlls/winspool.drv/info.c b/dlls/winspool.drv/info.c
index 54ecb81d3be..26e662d7ffb 100644
--- a/dlls/winspool.drv/info.c
+++ b/dlls/winspool.drv/info.c
@@ -1929,6 +1929,7 @@ BOOL WINAPI IsValidDevmodeW(PDEVMODEW dm, SIZE_T size)
     int i;
 
     if (!dm) return FALSE;
+    if (!dm->dmSize || size < dm->dmSize) return FALSE;
     if (size < FIELD_OFFSET(DEVMODEW, dmFields) + sizeof(dm->dmFields)) return FALSE;
 
     for (i = 0; i < ARRAY_SIZE(map); i++)
diff --git a/dlls/winspool.drv/tests/info.c b/dlls/winspool.drv/tests/info.c
index 92b042e354a..b6adaface65 100644
--- a/dlls/winspool.drv/tests/info.c
+++ b/dlls/winspool.drv/tests/info.c
@@ -3065,6 +3065,15 @@ static void test_IsValidDevmodeW(void)
         ret = IsValidDevmodeW(&dm, dm.dmSize);
         ok(ret == test[i].ret, "%d: got %d\n", i, ret);
     }
+
+    dm.dmSize = 0;
+
+    ret = IsValidDevmodeW(&dm, FIELD_OFFSET(DEVMODEW, u2.dmNup) + 4);
+    ok(!ret, "%d: got %d\n", i, ret); /* failed if dm.dmSize is zero */
+
+    dm.dmSize = FIELD_OFFSET(DEVMODEW, u2.dmNup) + 6;
+    ret = IsValidDevmodeW(&dm, FIELD_OFFSET(DEVMODEW, u2.dmNup) + 4);
+    ok(!ret, "%d: got %d\n", i, ret); /* failed if dm.dmSize greater than bufSize */
 }
 
 START_TEST(info)
-- 
GitLab

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

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/winspool.drv/tests/info.c:
> ret = IsValidDevmodeW(&dm, dm.dmSize);
>          ok(ret == test[i].ret, "%d: got %d\n", i, ret);
>      }
> +
> +    dm.dmSize = 0;
> +

It would be nice to remove this blank line, otherwise this looks good.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2262#note_25272