[PATCH v3 0/1] MR10785: msvcrt: Only reject negative timestamps with EINVAL in ctime_s
0 needs to be an allowed value, and negative values should not halt the program but just return EINVAL. This behavior has been observed with the following python script: ```python import ctypes ucrt = ctypes.CDLL("ucrtbase.dll") buf = ctypes.create_string_buffer(26) for i in [1, 0, -1]: print(f"_ctime64_s for time {i}:") t = ctypes.c_int64(i) err = ucrt._ctime64_s(buf, ctypes.c_size_t(26), ctypes.byref(t)) if err == 0: print(buf.value.decode()) else: print(f"Error: {err}") for i in [1, 0, -1]: print(f"_ctime32_s for time {i}:") t = ctypes.c_int32(i) err = ucrt._ctime32_s(buf, ctypes.c_size_t(26), ctypes.byref(t)) if err == 0: print(buf.value.decode()) else: print(f"Error: {err}") # _ctime64_s for time 1: # Thu Jan 1 01:00:01 1970 # # _ctime64_s for time 0: # Thu Jan 1 01:00:00 1970 # # _ctime64_s for time -1: # Error: 22 # # _ctime32_s for time 1: # Thu Jan 1 01:00:01 1970 # # _ctime32_s for time 0: # Thu Jan 1 01:00:00 1970 # # _ctime32_s for time -1: # Error: 22 ``` The bug https://bugs.winehq.org/show_bug.cgi?id=57261 is solved by this, because mongodb tries to pretty-print a 0 timestamp, and sets up its own invalid_parameter_handler, which crashes due to NULL arguments. -- v3: msvcrt: Only reject negative timestamps with EINVAL in ctime_s https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
From: Hendrik Borchardt <hendrik.borchardt@gmail.com> 0 needs to be an allowed value, and negative values should not halt the program but just return EINVAL. --- dlls/msvcrt/tests/time.c | 53 ++++++++++++++++++++++++++++++++++++++++ dlls/msvcrt/time.c | 10 ++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/dlls/msvcrt/tests/time.c b/dlls/msvcrt/tests/time.c index aba911f30a4..3c484159f26 100644 --- a/dlls/msvcrt/tests/time.c +++ b/dlls/msvcrt/tests/time.c @@ -51,6 +51,8 @@ typedef struct { int refcount; } __lc_time_data; +static errno_t (__cdecl *p_ctime32_s)(char*, size_t, __time32_t*); +static errno_t (__cdecl *p_ctime64_s)(char*, size_t, __time64_t*); static __time32_t (__cdecl *p_mkgmtime32)(struct tm*); static struct tm* (__cdecl *p_gmtime32)(__time32_t*); static struct tm* (__cdecl *p_gmtime)(time_t*); @@ -76,6 +78,8 @@ static void init(void) HMODULE hmod = LoadLibraryA("msvcrt.dll"); p_gmtime32 = (void*)GetProcAddress(hmod, "_gmtime32"); + p_ctime32_s = (void*)GetProcAddress(hmod, "_ctime32_s"); + p_ctime64_s = (void*)GetProcAddress(hmod, "_ctime64_s"); p_gmtime = (void*)GetProcAddress(hmod, "gmtime"); p_gmtime32_s = (void*)GetProcAddress(hmod, "_gmtime32_s"); p_gmtime64 = (void*)GetProcAddress(hmod, "_gmtime64"); @@ -116,6 +120,53 @@ static void test_ctime(void) ret = ctime(&badtime); ok(ret == NULL, "expected ctime to return NULL, got %s\n", ret); } + +static void test_ctime32_s(void) +{ + __time32_t goodtime = 0; + __time32_t badtime = -1; + char out[26]; + int ret; + + if(!p_ctime32_s) { + win_skip("Skipping _ctime32_s tests\n"); + return; + } + + errno = 0; + ret = p_ctime32_s(out, 26, &goodtime); + ok(ret == 0, "expected _ctime32_s to return NULL, got %d\n", ret); + ok(errno == 0, "expected _ctime32_s to set errno to 0, got %d\n", errno); + + errno = 0; + ret = p_ctime32_s(out, 26, &badtime); + ok(ret == EINVAL, "expected _ctime32_s to return EINVAL, got %d\n", ret); + ok(errno == EINVAL, "expected _ctime32_s to set errno to EINVAL, got %d\n", errno); +} + +static void test_ctime64_s(void) +{ + __time64_t goodtime = 0; + __time64_t badtime = -1; + char out[26]; + int ret; + + if(!p_ctime64_s) { + win_skip("Skipping _ctime64_s tests\n"); + return; + } + + errno = 0; + ret = p_ctime64_s(out, 26, &goodtime); + ok(ret == 0, "expected _ctime64_s to return NULL, got %d\n", ret); + ok(errno == 0, "expected _ctime64_s to set errno to 0, got %d\n", errno); + + errno = 0; + ret = p_ctime64_s(out, 26, &badtime); + ok(ret == EINVAL, "expected _ctime64_s to return EINVAL, got %d\n", ret); + ok(errno == EINVAL, "expected _ctime64_s to set errno to EINVAL, got %d\n", errno); +} + static void test_gmtime(void) { __time32_t valid, gmt; @@ -1038,6 +1089,8 @@ START_TEST(time) test__tzset(); test_strftime(); test_ctime(); + test_ctime32_s(); + test_ctime64_s(); test_gmtime(); test_gmtime64(); test_mktime(); diff --git a/dlls/msvcrt/time.c b/dlls/msvcrt/time.c index b52bb872647..92939c6b1a1 100644 --- a/dlls/msvcrt/time.c +++ b/dlls/msvcrt/time.c @@ -1737,7 +1737,10 @@ errno_t CDECL _ctime64_s(char *res, size_t len, const __time64_t *time) if (!MSVCRT_CHECK_PMT( len >= 26 )) return EINVAL; res[0] = '\0'; if (!MSVCRT_CHECK_PMT( time != NULL )) return EINVAL; - if (!MSVCRT_CHECK_PMT( *time > 0 )) return EINVAL; + if (*time < 0) { + *_errno() = EINVAL; + return EINVAL; + } ret = _localtime64_s( &t, time ); if (ret) @@ -1768,7 +1771,10 @@ errno_t CDECL _ctime32_s(char *res, size_t len, const __time32_t *time) if (!MSVCRT_CHECK_PMT( len >= 26 )) return EINVAL; res[0] = '\0'; if (!MSVCRT_CHECK_PMT( time != NULL )) return EINVAL; - if (!MSVCRT_CHECK_PMT( *time > 0 )) return EINVAL; + if (*time < 0) { + *_errno() = EINVAL; + return EINVAL; + } ret = _localtime32_s( &t, time ); if (ret) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
On Thu Apr 30 12:32:50 2026 +0000, Hendrik Borchardt wrote:
Sure, will do. But maybe this needs to be changed in the wiki? https://gitlab.winehq.org/wine/wine/-/wikis/Submitting-Patches#patch-guideli...: "If you've written a fix and a lot of new [conformance tests](Conformance-Tests) for an issue, add the tests as a separate commit ~~before~~after the fix" You can introduce the test before implementing the fix. Failing tests needs to be marked with `todo_wine`. Then, in the commit that adds the missing functionality, remove the `todo_wine` markers from those tests.
This change is easy to understand/review. In such case I don't mind if the tests are introduced first. The tests need to pass after every commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785#note_138376
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
On Thu Apr 30 13:23:41 2026 +0000, Piotr Caban wrote:
You can introduce the test before implementing the fix. Failing tests needs to be marked with `todo_wine`. Then, in the commit that adds the missing functionality, remove the `todo_wine` markers from those tests. This change is easy to understand/review. In such case I don't mind if the tests are introduced first. The tests need to pass after every commit. I've tried to edit that page to clarify the guidelines a bit more.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785#note_138414
participants (4)
-
Elizabeth Figura (＠zfigura) -
Hendrik Borchardt
-
Hendrik Borchardt (＠hborchardt)
-
Piotr Caban (＠piotr)