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 format specifier #4840

Merged
53 changes: 50 additions & 3 deletions stl/inc/xloctime
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,17 @@ protected:
__CLR_OR_THIS_CALL ~time_get_byname() noexcept override {}
};

constexpr bool _Is_valid_fmt_specifier(const char _Specifier) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
constexpr char _Valid_specifiers[] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ";
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
for (const char _Valid_specifier : _Valid_specifiers) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
if (_Valid_specifier == _Specifier) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
}

return false;
}
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 @@ -687,7 +698,19 @@ public:
_Specifier = _Ctype_fac.narrow(*_Fmtfirst);
}

_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Specifier == '%' && _Modifier == '\0') {
// if the specifier is percent and no modifier is set, just append it
*_Dest++ = _Percent;
} else if (!_Is_valid_fmt_specifier(_Specifier)) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
// no valid specifier, directly copy as literal elements
*_Dest++ = _Percent;
if (_Modifier != '\0') {
*_Dest++ = _Modifier;
}
*_Dest++ = _Specifier;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
}
}
}

Expand Down Expand Up @@ -811,7 +834,19 @@ public:
_Specifier = _Ctype_fac.narrow(*_Fmtfirst);
}

_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Specifier == '%' && _Modifier == '\0') {
// if the specifier is percent and no modifier is set, just append it
*_Dest++ = _Percent;
} else if (!_Is_valid_fmt_specifier(_Specifier)) {
// no valid specifier, directly copy as literal elements
*_Dest++ = _Percent;
*_Dest++ = _Raw;
if (_Modifier != '\0') {
*_Dest++ = *_Fmtfirst;
}
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
}
}
}

Expand Down Expand Up @@ -943,7 +978,19 @@ public:
_Specifier = _Ctype_fac.narrow(*_Fmtfirst);
}

_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
if (_Specifier == '%' && _Modifier == '\0') {
// if the specifier is percent and no modifier is set, just append it
*_Dest++ = _Percent;
} else if (!_Is_valid_fmt_specifier(_Specifier)) {
// no valid specifier, directly copy as literal elements
*_Dest++ = _Percent;
if (_Modifier != '\0') {
*_Dest++ = _Modifier;
}
*_Dest++ = _Specifier;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
}
}
}

Expand Down
28 changes: 26 additions & 2 deletions tests/std/tests/Dev11_0836436_get_time/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ void test_invalid_argument();
void test_buffer_resizing();
void test_gh_2618();
void test_gh_2848();
void test_gh_4820();

int main() {
assert(read_hour("12 AM") == 0);
Expand Down Expand Up @@ -157,6 +158,7 @@ int main() {
test_buffer_resizing();
test_gh_2618();
test_gh_2848();
test_gh_4820();
}

typedef istreambuf_iterator<char> Iter;
Expand Down Expand Up @@ -792,16 +794,17 @@ void test_invalid_argument() {
time_t t = time(nullptr);
tm currentTime;
localtime_s(&currentTime, &t);
currentTime.tm_hour = 25; // set invalid hour
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

{
wstringstream wss;
wss << put_time(&currentTime, L"%Y-%m-%d-%H-%M-%s");
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-%s");
ss << put_time(&currentTime, "%Y-%m-%d-%H-%M");
assert(ss.rdstate() == ios_base::badbit);
}
#endif // _M_CEE_PURE
Expand Down Expand Up @@ -905,3 +908,24 @@ void test_gh_2848() {
assert(err == (ios_base::eofbit | ios_base::failbit));
}
}

void test_gh_4820() {
// GH-4820 <iomanip>: std::put_time should copy unknown conversion specifiers instead of crash
time_t t = time(nullptr);
tm currentTime;
localtime_s(&currentTime, &t);

{
wstringstream wss;
wss << put_time(&currentTime, L"%Ei%!%E%J%P");
assert(wss.rdstate() == ios_base::goodbit);
assert(wss.str() == L"%Ei%!%E%J%P");
}

{
stringstream ss;
ss << put_time(&currentTime, "%Ei%!%E%J%P");
assert(ss.rdstate() == ios_base::goodbit);
assert(ss.str() == "%Ei%!%E%J%P");
}
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}