Skip to content

Commit

Permalink
Fix put_time() crash on invalid format specifier (#4840)
Browse files Browse the repository at this point in the history 
Co-authored-by: Stephan T. Lavavej <[email protected]>
  • Loading branch information
@TheStormN @StephanTLavavej
TheStormN and StephanTLavavej authored Aug 8, 2024
1 parent c4d0517 commit 0f0f3aa
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 7 deletions.
62 changes: 57 additions & 5 deletions stl/inc/xloctime
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,21 @@ protected:
__CLR_OR_THIS_CALL ~time_get_byname() noexcept override {}
};

// C23 7.29.3.5 "The strftime function"/3
_INLINE_VAR constexpr char _Valid_strftime_specifiers[] = {'a', 'A', 'b', 'B', 'c', 'C', 'd', 'D', 'e', 'F', 'g', 'G',
'h', 'H', 'I', 'j', 'm', 'M', 'n', 'p', 'r', 'R', 'S', 't', 'T', 'u', 'U', 'V', 'w', 'W', 'x', 'X', 'y', 'Y', 'z',
'Z'};

_NODISCARD constexpr bool _Is_valid_strftime_specifier(const char _Specifier) {
for (const auto& _Valid_specifier : _Valid_strftime_specifiers) {
if (_Specifier == _Valid_specifier) {
return true;
}
}

return false;
}

_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 +702,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_strftime_specifier(_Specifier)) {
// 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 +838,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_strftime_specifier(_Specifier)) {
// no valid specifier, directly copy as literal elements
*_Dest++ = _Percent;
if (_Modifier != '\0') {
*_Dest++ = _Raw;
}
*_Dest++ = *_Fmtfirst;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
}
}
}

Expand Down Expand Up @@ -927,14 +966,15 @@ public:
*_Dest++ = _Fmtfirst[-1];
break;
} else { // get specifier after %
char _Specifier = _Ctype_fac.narrow(*_Fmtfirst);
_Elem _Raw = *_Fmtfirst;
char _Specifier = _Ctype_fac.narrow(_Raw);
char _Modifier = '\0';
_Elem _Percent = _Fmtfirst[-1];

if (_Specifier == 'E' || _Specifier == 'O' || _Specifier == 'Q' || _Specifier == '#') {
if (++_Fmtfirst == _Fmtlast) { // no specifier, copy %[E0Q#] as literal elements
*_Dest++ = _Percent;
*_Dest++ = _Specifier;
*_Dest++ = _Raw;
break;
}

Expand All @@ -943,7 +983,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_strftime_specifier(_Specifier)) {
// no valid specifier, directly copy as literal elements
*_Dest++ = _Percent;
if (_Modifier != '\0') {
*_Dest++ = _Raw;
}
*_Dest++ = *_Fmtfirst;
} else {
_Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
}
}
}

Expand Down
43 changes: 41 additions & 2 deletions tests/std/tests/Dev11_0836436_get_time/test.cpp
View file
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

{
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,39 @@ 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);

// Case 1: Test various unknown conversion specifiers.
// Case 2: "%%" is a known escape sequence with a dedicated fast path.
// Case 3: "% " is percent followed by space, which is an unknown conversion specifier.
// Case 4: "%E%Z" is parsed as "%E%" followed by "Z", so it should be copied unchanged,
// even though "%Z" by itself would be a known conversion specifier (time zone name).
// (In case 1, "%E%J" is parsed the same way; the difference is that "%J" would be unknown.)
{
wstringstream wss;
wss << put_time(&currentTime, L"1:%Ei%!%E%J%P 2:%% 3:% 4:%E%Z");
assert(wss.rdstate() == ios_base::goodbit);
assert(wss.str() == L"1:%Ei%!%E%J%P 2:% 3:% 4:%E%Z");
}

{
stringstream ss;
ss << put_time(&currentTime, "1:%Ei%!%E%J%P 2:%% 3:% 4:%E%Z");
assert(ss.rdstate() == ios_base::goodbit);
assert(ss.str() == "1:%Ei%!%E%J%P 2:% 3:% 4:%E%Z");
}

// Also verify that wide characters aren't truncated.
// This tests a character appearing by itself, two as specifiers, and two as modified specifiers.
{
wstringstream wss;
wss << put_time(&currentTime, L"\x043a%\x043e%\x0448%E\x043a%O\x0430");
assert(wss.rdstate() == ios_base::goodbit);
assert(wss.str() == L"\x043a%\x043e%\x0448%E\x043a%O\x0430");
}
}

0 comments on commit 0f0f3aa

Please sign in to comment.