Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix put_time() crash on invalid struct tm data #4883

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 118 additions & 3 deletions stl/inc/xloctime
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,109 @@ _NODISCARD constexpr bool _Is_valid_strftime_specifier(const char _Specifier) {
return false;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_sec(const tm* const _Pt) noexcept {
// seconds after the minute - [0, 60] including leap second
return _Pt->tm_sec >= 0 && _Pt->tm_sec <= 60;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_min(const tm* const _Pt) noexcept {
// minutes after the hour - [0, 59]
return _Pt->tm_min >= 0 && _Pt->tm_min <= 59;
}
TheStormN marked this conversation as resolved.
Show resolved Hide resolved

_NODISCARD constexpr bool _Is_valid_strftime_tm_hour(const tm* const _Pt) noexcept {
// hours since midnight - [0, 23]
return _Pt->tm_hour >= 0 && _Pt->tm_hour <= 23;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_mday(const tm* const _Pt) noexcept {
// day of the month - [1, 31]
return _Pt->tm_mday >= 1 && _Pt->tm_mday <= 31;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_mon(const tm* const _Pt) noexcept {
// months since January - [0, 11]
return _Pt->tm_mon >= 0 && _Pt->tm_mon <= 11;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_year(const tm* const _Pt) noexcept {
// years since 1900 - UCRT max range is up until 8099
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
return _Pt->tm_year >= -1900 && _Pt->tm_year <= 8099;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_wday(const tm* const _Pt) noexcept {
// days since Sunday - [0, 6]
return _Pt->tm_wday >= 0 && _Pt->tm_wday <= 6;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_yday(const tm* const _Pt) noexcept {
// days since January 1 - [0, 365]
return _Pt->tm_yday >= 0 && _Pt->tm_yday <= 365;
}

_NODISCARD constexpr bool _Is_valid_strftime_tm_data(const char _Specifier, const tm* const _Pt) noexcept {
if (!_Pt) {
return false;
}

switch (_Specifier) {
case 'S':
return _Is_valid_strftime_tm_sec(_Pt);
case 'M':
return _Is_valid_strftime_tm_min(_Pt);
case 'H':
case 'I':
case 'p':
return _Is_valid_strftime_tm_hour(_Pt);
case 'd':
case 'e':
return _Is_valid_strftime_tm_mday(_Pt);
case 'b':
case 'B':
case 'm':
case 'h':
return _Is_valid_strftime_tm_mon(_Pt);
case 'C':
case 'y':
case 'Y':
return _Is_valid_strftime_tm_year(_Pt);
case 'j':
return _Is_valid_strftime_tm_yday(_Pt);
case 'a':
case 'A':
case 'u':
case 'w':
return _Is_valid_strftime_tm_wday(_Pt);
case 'U':
case 'W':
return _Is_valid_strftime_tm_wday(_Pt) && _Is_valid_strftime_tm_yday(_Pt);
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
case 'R':
return _Is_valid_strftime_tm_hour(_Pt) && _Is_valid_strftime_tm_min(_Pt);
case 'D':
case 'x':
case 'F':
return _Is_valid_strftime_tm_year(_Pt) && _Is_valid_strftime_tm_mon(_Pt) && _Is_valid_strftime_tm_mday(_Pt);
case 'g':
case 'G':
return _Is_valid_strftime_tm_year(_Pt) && _Is_valid_strftime_tm_wday(_Pt) && _Is_valid_strftime_tm_yday(_Pt);
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
case 'r':
case 'X':
case 'T':
return _Is_valid_strftime_tm_hour(_Pt) && _Is_valid_strftime_tm_min(_Pt) && _Is_valid_strftime_tm_sec(_Pt);
case 'c':
return _Is_valid_strftime_tm_wday(_Pt) && _Is_valid_strftime_tm_mon(_Pt) && _Is_valid_strftime_tm_mday(_Pt)
&& _Is_valid_strftime_tm_hour(_Pt) && _Is_valid_strftime_tm_min(_Pt) && _Is_valid_strftime_tm_sec(_Pt)
&& _Is_valid_strftime_tm_year(_Pt);
default:
break;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}

// daylight savings time flag
// _Pt->tm_isdst - no need for validation as UCRT treats it as boolean

return true;
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}

_EXPORT_STD extern "C++" template <class _Elem, class _OutIt = ostreambuf_iterator<_Elem, char_traits<_Elem>>>
class time_put : public locale::facet { // facet for converting encoded times to text
public:
Expand Down Expand Up @@ -713,7 +816,11 @@ public:
}
*_Dest++ = _Specifier;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Is_valid_strftime_tm_data(_Specifier, _Pt)) {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
} else {
*_Dest++ = _Elem('?');
}
}
}
}
Expand Down Expand Up @@ -849,7 +956,11 @@ public:
}
*_Dest++ = *_Fmtfirst;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Is_valid_strftime_tm_data(_Specifier, _Pt)) {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
} else {
*_Dest++ = _Elem('?');
}
}
}
}
Expand Down Expand Up @@ -994,7 +1105,11 @@ public:
}
*_Dest++ = *_Fmtfirst;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Is_valid_strftime_tm_data(_Specifier, _Pt)) {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
} else {
*_Dest++ = _Elem('?');
}
}
}
}
Expand Down
110 changes: 72 additions & 38 deletions tests/std/tests/Dev11_0836436_get_time/test.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <algorithm>
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
#include <cassert>
#include <cstdio>
#include <cstdlib>
Expand Down Expand Up @@ -106,11 +107,11 @@ void test_DevDiv_990695();
void test_locale_russian();
void test_locale_german();
void test_locale_chinese();
void test_invalid_argument();
void test_buffer_resizing();
void test_gh_2618();
void test_gh_2848();
void test_gh_4820();
void test_gh_4882();

int main() {
assert(read_hour("12 AM") == 0);
Expand Down Expand Up @@ -154,11 +155,11 @@ int main() {
test_locale_russian();
test_locale_german();
test_locale_chinese();
test_invalid_argument();
test_buffer_resizing();
test_gh_2618();
test_gh_2848();
test_gh_4820();
test_gh_4882();
}

typedef istreambuf_iterator<char> Iter;
Expand Down Expand Up @@ -774,42 +775,6 @@ void test_locale_chinese() {
assert(read_date_locale(L"2020-\x0031\x0032\x6708-31", "zh-CN") == make_tuple(31, 11, 120));
}

void test_invalid_parameter_handler(const wchar_t* const expression, const wchar_t* const function,
const wchar_t* const file, const unsigned int line, const uintptr_t reserved) {
(void) expression;
(void) reserved;

static int num_called = 0;
if (++num_called > 10) {
wprintf(
L"Test Failed: Invalid parameter handler was called over 10 times by %s in %s:%u\n", function, file, line);
exit(1);
}
}

void test_invalid_argument() {
#ifndef _M_CEE_PURE
_set_invalid_parameter_handler(test_invalid_parameter_handler);

time_t t = time(nullptr);
tm currentTime;
localtime_s(&currentTime, &t);
currentTime.tm_hour = 25; // set invalid hour

{
wstringstream wss;
wss << put_time(&currentTime, L"%Y-%m-%d-%H-%M");
assert(wss.rdstate() == ios_base::badbit);
}

{
stringstream ss;
ss << put_time(&currentTime, "%Y-%m-%d-%H-%M");
assert(ss.rdstate() == ios_base::badbit);
}
#endif // _M_CEE_PURE
}

void test_buffer_resizing() {
time_t t = time(nullptr);
tm currentTime;
Expand Down Expand Up @@ -944,3 +909,72 @@ void test_gh_4820() {
assert(wss.str() == L"\x043a%\x043e%\x0448%E\x043a%O\x0430");
}
}

void test_gh_4882() {
// GH-4882 <iomanip>: std::put_time should not crash on invalid/out-of-range tm struct values
const auto fieldValidation = [](int tm::*field, int value, const string& format) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
time_t t = time(nullptr);
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
tm currentTime;
localtime_s(&currentTime, &t);

(currentTime.*field) = value;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

stringstream ss;
ss << put_time(&currentTime, format.c_str());
assert(ss.rdstate() == ios_base::goodbit);
const auto result = ss.str();
assert(result.size() == format.size() / 2);
assert(all_of(result.cbegin(), result.cend(), [](const char c) { return c == '?'; }));

// Narrow conversion is good enough for our ASCII only format strings
wstring wformat(format.size(), L' ');
transform(
format.cbegin(), format.cend(), wformat.begin(), [](const char c) { return static_cast<wchar_t>(c); });

wstringstream wss;
wss << put_time(&currentTime, wformat.c_str());
assert(wss.rdstate() == ios_base::goodbit);
const auto wresult = wss.str();
assert(wresult.size() == format.size() / 2);
assert(all_of(wresult.cbegin(), wresult.cend(), [](const wchar_t c) { return c == L'?'; }));
};

struct FormatTestData {
int tm::*field;
int value;
string format;
};

const FormatTestData testDataList[] = {
{&tm::tm_sec, -1, "%S%r%X%T%c"},
{&tm::tm_sec, 61, "%S%r%X%T%c"},
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

{&tm::tm_min, -1, "%M%r%X%T%c"},
{&tm::tm_min, 60, "%M%r%X%T%c"},
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

{&tm::tm_hour, -1, "%H%I%p%R%r%X%T%c"},
{&tm::tm_hour, 24, "%H%I%p%R%r%X%T%c"},

{&tm::tm_mday, 0, "%d%e%c"},
{&tm::tm_mday, 32, "%d%e%c"},

{&tm::tm_mon, -1, "%b%B%m%h%c"},
{&tm::tm_mon, 12, "%b%B%m%h%c"},
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

{&tm::tm_year, -1901, "%C%y%Y%D%x%F%g%G%c"},
{&tm::tm_year, 8100, "%C%y%Y%D%x%F%g%G%c"},

{&tm::tm_yday, -1, "%j%U%W"},
{&tm::tm_yday, 366, "%j%U%W"},

{&tm::tm_wday, -1, "%a%A%u%w%c"},
{&tm::tm_wday, 7, "%a%A%u%w%c"},

{&tm::tm_wday, -1, "%U%W"},
{&tm::tm_wday, 7, "%U%W"},
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
};

for (const auto& testData : testDataList) {
fieldValidation(testData.field, testData.value, testData.format);
}
}