Fix std::put_time() crash on invalid format specifier

microsoft · Jul 14, 2024 · c316691 · c316691
1 parent ecbc1ef
commit c316691
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 5 deletions.
diff --git a/stl/inc/xloctime b/stl/inc/xloctime
@@ -651,6 +651,17 @@ protected:
     __CLR_OR_THIS_CALL ~time_get_byname() noexcept override {}
 };
 
+constexpr bool _Is_valid_fmt_specifier(const char _Specifier) {
+    constexpr char _Valid_specifiers[] = "aAbBcCdDeFgGhHIjmMnprRStTuUVwWxXyYzZ";
+    for (const char _Valid_specifier : _Valid_specifiers) {
+        if (_Valid_specifier == _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:
@@ -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)) {
+                    // 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
+                }
             }
         }
 
@@ -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;
+                    }
+                } else {
+                    _Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
+                }
             }
         }
 
@@ -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;
+                } else {
+                    _Dest = do_put(_Dest, _Iosbase, _Fill, _Pt, _Specifier, _Modifier); // convert a single field
+                }
             }
         }
 

diff --git a/tests/std/tests/Dev11_0836436_get_time/test.cpp b/tests/std/tests/Dev11_0836436_get_time/test.cpp
@@ -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);
@@ -157,6 +158,7 @@ int main() {
     test_buffer_resizing();
     test_gh_2618();
     test_gh_2848();
+    test_gh_4820();
 }
 
 typedef istreambuf_iterator<char> Iter;
@@ -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
@@ -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");
+    }
+}