[PATCH v4 0/1] MR3358: msvcrt: fix out-of-bound access in create_locinfo

[Yuxuan Shui (＠yshui)]

`wcsncpy` and `GetLocaleInfoEx` take length in number of characters, but `size` and `ret` counts number of bytes.

Previous commit changed a call to `GetLocaleInfoW` which counts lenght in `TCHAR`s (aka bytes), to the current `GetLocaleInfoEx`, which is probably the source of this confusion.

--
  v4: msvcrt: Fix out-of-bound access in create_locinfo.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3358

[PATCH v4 1/1] msvcrt: Fix out-of-bound access in create_locinfo.

[Yuxuan Shui]

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

Fixes regression introduced by 24a2b625545f1875b5c3177f2b9.

Signed-off-by: Yuxuan Shui <yshui(a)codeweavers.com>
---
 dlls/msvcrt/locale.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 9fa6ba76143..10682990bfc 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -1226,7 +1226,7 @@ static __lc_time_data* create_time_data(WCHAR *sname)
     int i, ret, size;
     LCID lcid = LocaleNameToLCID(sname, LCID_CONVERSION_FLAGS);
 
-    size = sizeof(__lc_time_data);
+    size = 0;
     for(i=0; i<ARRAY_SIZE(time_data); i++) {
         ret = GetLocaleInfoA(lcid, time_data[i], NULL, 0);
         if(!ret)
@@ -1241,10 +1241,10 @@ static __lc_time_data* create_time_data(WCHAR *sname)
 #endif
     }
 #if _MSVCR_VER >= 110
-    size += wcslen(sname)*sizeof(wchar_t);
+    size += (wcslen(sname) + 1) * sizeof(wchar_t);
 #endif
 
-    cur = malloc(size);
+    cur = malloc(FIELD_OFFSET(__lc_time_data, data[size]));
     if(!cur)
         return NULL;
 
@@ -1256,13 +1256,13 @@ static __lc_time_data* create_time_data(WCHAR *sname)
 #if _MSVCR_VER == 0 || _MSVCR_VER >= 100
     for(i=0; i<ARRAY_SIZE(time_data); i++) {
         cur->wstr.wstr[i] = (wchar_t*)&cur->data[ret];
-        ret += GetLocaleInfoEx(sname, time_data[i],
-                (wchar_t*)&cur->data[ret], size-ret)*sizeof(wchar_t);
+        ret += GetLocaleInfoEx(sname, time_data[i], (wchar_t*)&cur->data[ret],
+                (size - ret) / sizeof(wchar_t)) * sizeof(wchar_t);
     }
 #endif
 #if _MSVCR_VER >= 110
     cur->locname = (wchar_t*)&cur->data[ret];
-    wcsncpy((wchar_t *) &cur->data[ret], sname, size-ret);
+    wcscpy((wchar_t*)&cur->data[ret], sname);
 #else
     cur->lcid = lcid;
 #endif
-- 
GitLab

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

[Piotr Caban (＠piotr)]

I've pushed updated version of the patch that changes `size` meaning. It's now used to store size of data following `__lc_time_data` struct.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3358#note_39623

[Yuxuan Shui (＠yshui)]

On Thu Jul 20 14:49:04 2023 +0000, Piotr Caban wrote:
> I've pushed updated version of the patch that changes `size` meaning.
> It's now used to store size of data following `__lc_time_data` struct.
> LGTM :thumbsup:

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3358#note_39624

[PATCH v4 0/1] MR3358: msvcrt: fix out-of-bound access in create_locinfo - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3358