ASan should detect writing to a basic_string's reserved but uninitialized memory (#5252)

davidmrdavid · StephanTLavavej · web-flow · commit d655c17a7e58 · 2025-01-29T11:54:51.000-08:00
Co-authored-by: David Justo &lt;dajusto@microsoft.com&gt;
Co-authored-by: Stephan T. Lavavej &lt;stl@microsoft.com&gt;
diff --git a/stl/inc/xstring b/stl/inc/xstring
@@ -2418,6 +2418,10 @@ public:
             [](_Elem* const _New_ptr, const _Elem* const _Old_ptr, const size_type _Old_size)
                 _STATIC_CALL_OPERATOR { _Traits::copy(_New_ptr, _Old_ptr, _Old_size + 1); });
 
+        // `_Reallocate_grow_by` calls `_ASAN_STRING_CREATE` assuming that the string
+        // has size (initialized memory) equal to its new capacity (allocated memory).
+        // This is not true for the `reserve` method, so we modify the ASan annotation.
+        _ASAN_STRING_MODIFY(*this, _Mypair._Myval2._Mysize, _Old_size);
         _Mypair._Myval2._Mysize = _Old_size;
     }
 
@@ -2442,6 +2446,10 @@ public:
                 [](_Elem* const _New_ptr, const _Elem* const _Old_ptr, const size_type _Old_size)
                     _STATIC_CALL_OPERATOR { _Traits::copy(_New_ptr, _Old_ptr, _Old_size + 1); });
 
+            // `_Reallocate_grow_by` calls `_ASAN_STRING_CREATE` assuming that the string
+            // has size (initialized memory) equal to its new capacity (allocated memory).
+            // This is not true for the `reserve` method, so we modify the ASan annotation.
+            _ASAN_STRING_MODIFY(*this, _Mypair._Myval2._Mysize, _Old_size);
             _Mypair._Myval2._Mysize = _Old_size;
             return;
         }
diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp
@@ -20,6 +20,8 @@
 #include <new>
 #include <sstream>
 #include <string>
+
+#include <test_death.hpp>
 #if _HAS_CXX17
 #include <string_view>
 #endif // _HAS_CXX17
@@ -1917,17 +1919,32 @@ void test_gh_3955() {
     assert(s == t);
 }
 
-int main() {
-    run_allocator_matrix<char>();
+void test_gh_5251() {
+    // GH-5251 <string>: ASan annotations do not prevent writing to allocated
+    // but uninitialized basic_string memory
+    string myString;
+    myString.reserve(100);
+    char* myData = &myString[0];
+    myData[50]   = 'A'; // ASan should fire!
+}
+
+int main(int argc, char* argv[]) {
+    std_testing::death_test_executive exec([] {
+        run_allocator_matrix<char>();
 #ifdef __cpp_char8_t
-    run_allocator_matrix<char8_t>();
+        run_allocator_matrix<char8_t>();
 #endif // __cpp_char8_t
-    run_allocator_matrix<char16_t>();
-    run_allocator_matrix<char32_t>();
-    run_allocator_matrix<wchar_t>();
-
-    test_DevCom_10116361();
-    test_DevCom_10109507();
-    test_gh_3883();
-    test_gh_3955();
+        run_allocator_matrix<char16_t>();
+        run_allocator_matrix<char32_t>();
+        run_allocator_matrix<wchar_t>();
+
+        test_DevCom_10116361();
+        test_DevCom_10109507();
+        test_gh_3883();
+        test_gh_3955();
+    });
+#ifdef __SANITIZE_ADDRESS__
+    exec.add_death_tests({test_gh_5251});
+#endif // ASan instrumentation enabled
+    return exec.run(argc, argv);
 }
