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 6 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
106 changes: 103 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,94 @@ _NODISCARD constexpr bool _Is_valid_strftime_specifier(const char _Specifier) {
return false;
}

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

switch (_Specifier) {
case 'S':
// seconds after the minute - [0, 60] including leap second
if (_Pt->tm_sec < 0 || _Pt->tm_sec > 60) {
return false;
}
break;
case 'M':
// minutes after the hour - [0, 59]
if (_Pt->tm_min < 0 || _Pt->tm_min > 59) {
return false;
}
break;
case 'H':
case 'I':
case 'p':
// hours since midnight - [0, 23]
if (_Pt->tm_hour < 0 || _Pt->tm_hour > 23) {
return false;
}
break;
case 'd':
case 'e':
// day of the month - [1, 31]
if (_Pt->tm_mday < 1 || _Pt->tm_mday > 31) {
return false;
}
break;
case 'b':
case 'B':
case 'm':
// months since January - [0, 11]
if (_Pt->tm_mon < 0 || _Pt->tm_mon > 11) {
return false;
}
break;
case 'C':
case 'g':
case 'G':
case 'y':
case 'Y':
// years since 1900 - UCRT max range is up until 8099
if (_Pt->tm_year < -1900 || _Pt->tm_year > 8099) {
return false;
}
break;
case 'a':
case 'A':
case 'u':
case 'w':
// days since Sunday - [0, 6]
if (_Pt->tm_wday < 0 || _Pt->tm_wday > 6) {
return false;
}
break;
case 'U':
case 'W':
// days since Sunday - [0, 6]
if (_Pt->tm_wday < 0 || _Pt->tm_wday > 6) {
return false;
}

// days since January 1 - [0, 365]
if (_Pt->tm_yday < 0 || _Pt->tm_yday > 365) {
return false;
}
break;
case 'j':
// days since January 1 - [0, 365]
if (_Pt->tm_yday < 0 || _Pt->tm_yday > 365) {
return false;
}
break;
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 +801,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 +941,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 +1090,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
106 changes: 68 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,68 @@ 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 set failbit on invalid/out-of-range tm struct, instead of crash
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
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.length() == format.length() / 2);
TheStormN marked this conversation as resolved.
Show resolved Hide resolved
assert(std::all_of(result.cbegin(), result.cend(), [](const char c) { return c == '?'; }));
TheStormN marked this conversation as resolved.
Show resolved Hide resolved

// Narrow conversion is good enough for our ASCII only format strings
wstring wformat(format.length(), L' ');
transform(format.cbegin(), format.cend(), wformat.begin(), [](const char c) { return wchar_t(c); });
TheStormN marked this conversation as resolved.
Show resolved Hide resolved

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

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

const FormatTestData testDataList[] = {
{&tm::tm_sec, -1, "%S"},
{&tm::tm_sec, 61, "%S"},

{&tm::tm_min, -1, "%M"},
{&tm::tm_min, 60, "%M"},

{&tm::tm_hour, -1, "%H%I%p"},
{&tm::tm_hour, 24, "%H%I%p"},

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

{&tm::tm_mon, -1, "%b%B%m"},
{&tm::tm_mon, 12, "%b%B%m"},

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

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

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

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