[PATCH v4 0/1] MR8125: mscoree: Add missing NUL terminator.
Fix out-of-bound access by wcslen. Found by ASan. -- v4: mscoree: Add missing NUL terminator. https://gitlab.winehq.org/wine/wine/-/merge_requests/8125
From: Yuxuan Shui <yshui(a)codeweavers.com> Fix out-of-bound access by wcslen. Found by ASan. --- dlls/mscoree/metahost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mscoree/metahost.c b/dlls/mscoree/metahost.c index e1dd00656e9..1628436ac6f 100644 --- a/dlls/mscoree/metahost.c +++ b/dlls/mscoree/metahost.c @@ -842,7 +842,7 @@ static BOOL get_mono_path_datadir(LPWSTR path) { static const WCHAR winedatadirW[] = {'W','I','N','E','D','A','T','A','D','I','R',0}; static const WCHAR winebuilddirW[] = {'W','I','N','E','B','U','I','L','D','D','I','R',0}; - static const WCHAR unix_prefix[] = {'\\','?','?','\\','u','n','i','x','\\'}; + static const WCHAR unix_prefix[] = {'\\','?','?','\\','u','n','i','x','\\',0}; static const WCHAR monoW[] = {'\\','m','o','n','o',0}; static const WCHAR dotdotmonoW[] = {'\\','.','.','\\','m','o','n','o',0}; const WCHAR *data_dir, *suffix; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8125
oh wait, this one is a `sizeof`, not strlen, so this one is fine. sorry about that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_104275
This should probably be a memcmp/sizeof like the other one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_104291
On Fri May 23 10:06:05 2025 +0000, Esme Povirk wrote:
This should probably be a memcmp/sizeof like the other one. wouldn't memcmp access out-of-bound if `data_dir` is not long enough? the same goes for the other one actually.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_104326
On Fri May 23 10:06:05 2025 +0000, Yuxuan Shui wrote:
wouldn't memcmp access out-of-bound if `data_dir` is not long enough? the same goes for the other one actually. `data_dir` should be null-terminated, so if it's not long enough then it should mismatch at the null terminator and memcmp should stop there.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_104457
`data_dir` should be null-terminated, so if it's not long enough then it should mismatch at the null terminator and memcmp should stop there.
I don't think that's how memcmp() works? The standard is a bit vague, but I'm inclined to read that it's allowed to read all <size> bytes of both memory areas. Certainly standard libraries tend to take advantage of that by doing comparisons of larger words at a time. I guess it's impossible for that to actually break, but it still seems like it should be more correct, even idiomatic, to actually check "strlen(dir) > sizeof(unix_prefix)" first. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_104484
I think this has been superseded by 96b8525de729093104b7b0bcd590b961bdf2c8af ? @madewokherd @julliard -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_107858
On Wed Aug 20 16:12:03 2025 +0000, Yuxuan Shui wrote:
I think this has been superseded by 96b8525de729093104b7b0bcd590b961bdf2c8af ? @madewokherd @julliard Yes, closing.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125#note_113294
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8125
participants (5)
-
Alexandre Julliard (＠julliard) -
Elizabeth Figura (＠zfigura)
-
Esme Povirk (＠madewokherd)
-
Yuxuan Shui
-
Yuxuan Shui (＠yshui)