[PATCH 0/1] MR8260: kernel32/tests: Fix out-of-bound read in test_CreateFileA.

[Yuxuan Shui (＠yshui)]

We check for patterns in test file names, but we failed to account for empty
and short names.

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

[PATCH 1/1] kernel32/tests: Fix out-of-bound read in test_CreateFileA.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

We check for patterns in test file names, but we failed to account for empty
and short names.
---
 dlls/kernel32/tests/file.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index cf0c40d5cd6..9d3d78c43dd 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -73,6 +73,7 @@ static const char sillytext[] =
 
 struct test_list {
     const char *file;           /* file string to test */
+    const SIZE_T file_len;      /* length of the file string */
     const DWORD err;            /* Win NT and further error code */
     const LONG err2;            /* Win 9x & ME error code  or -1 */
     const DWORD options;        /* option flag to use for open */
@@ -1275,23 +1276,25 @@ static void test_CreateFileA(void)
     char directory[] = "removeme";
     static const char nt_drive[] = "\\\\?\\A:";
     DWORD i, ret, len;
+#define X(str) str, sizeof(str) - 1
     static const struct test_list p[] =
     {
-        {"", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dir as file w \ */
-        {"", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* dir as dir w \ */
-        {"a", ERROR_FILE_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist file */
-        {"a\\", ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist dir */
-        {"removeme", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w/o \ */
-        {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exst dir w \ */
-        {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* device in file namespace */
-        {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* device in file namespace as dir */
-        {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, TRUE }, /* root dir w \ */
-        {"c:\\", ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* root dir w \ as dir */
-        {"c:c:\\windows", ERROR_INVALID_NAME, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* invalid path */
-        {"\\\\?\\c:", ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL,FALSE }, /* dev namespace drive */
-        {"\\\\?\\c:\\", ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dev namespace drive w \ */
-        {NULL, 0, -1, 0, FALSE}
+        {X(""), ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dir as file w \ */
+        {X(""), ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* dir as dir w \ */
+        {X("a"), ERROR_FILE_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist file */
+        {X("a\\"), ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist dir */
+        {X("removeme"), ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w/o \ */
+        {X("removeme\\"), ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exst dir w \ */
+        {X("c:"), ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* device in file namespace */
+        {X("c:"), ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* device in file namespace as dir */
+        {X("c:\\"), ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, TRUE }, /* root dir w \ */
+        {X("c:\\"), ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* root dir w \ as dir */
+        {X("c:c:\\windows"), ERROR_INVALID_NAME, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* invalid path */
+        {X("\\\\?\\c:"), ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL,FALSE }, /* dev namespace drive */
+        {X("\\\\?\\c:\\"), ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dev namespace drive w \ */
+        {NULL, 0, 0, -1, 0, FALSE}
     };
+#undef X
     BY_HANDLE_FILE_INFORMATION  Finfo;
     WCHAR curdir[MAX_PATH];
 
@@ -1365,12 +1368,12 @@ static void test_CreateFileA(void)
     {
         filename[0] = 0;
         /* update the drive id in the table entry with the current one */
-        if (p[i].file[1] == ':')
+        if (p[i].file_len > 1 && p[i].file[1] == ':')
         {
             strcpy(filename, p[i].file);
             filename[0] = windowsdir[0];
         }
-        else if (p[i].file[0] == '\\' && p[i].file[5] == ':')
+        else if (p[i].file_len > 5 && p[i].file[0] == '\\' && p[i].file[5] == ':')
         {
             strcpy(filename, p[i].file);
             filename[4] = windowsdir[0];
-- 
GitLab

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

[Nikolay Sivov (＠nsivov)]

It looks like it's enough to use strlen on it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8260#note_105934