Fix use-after-free in EventPipe (#81135)

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Elinor Fung <elfung@microsoft.com>

diff --git a/src/coreclr/inc/sstring.h b/src/coreclr/inc/sstring.h
index 491a6555da458ee69ea19649d390d931d3c09088..f5ebd793b97d59718e2df2c28062210fb27ad4e8 100644 (file)
--- a/src/coreclr/inc/sstring.h
+++ b/src/coreclr/inc/sstring.h
@@ -518,6 +518,9 @@ private:
     //Returns the unicode string, the caller is responsible for lifetime of the string
     WCHAR *GetCopyOfUnicodeString();
 
+    //Returns the UTF8 string, the caller is responsible for the lifetime of the string
+    UTF8 *GetCopyOfUTF8String();
+
     // Get the max size that can be passed to OpenUnicodeBuffer without causing allocations.
     COUNT_T GetUnicodeAllocation();
 

diff --git a/src/coreclr/inc/sstring.inl b/src/coreclr/inc/sstring.inl
index 2cc0d23c0bd23a99bfed142e1142550149123b36..b7743b0bd5955f0d9be206a22823b886a5acae30 100644 (file)
--- a/src/coreclr/inc/sstring.inl
+++ b/src/coreclr/inc/sstring.inl
@@ -1648,6 +1648,28 @@ inline WCHAR *SString::GetCopyOfUnicodeString()
     SS_RETURN buffer.Extract();
 }
 
+//----------------------------------------------------------------------------
+// Return a copy of the underlying  buffer, the caller is responsible for managing
+// the returned memory
+//----------------------------------------------------------------------------
+inline UTF8 *SString::GetCopyOfUTF8String()
+{
+    SS_CONTRACT(UTF8*)
+    {
+        GC_NOTRIGGER;
+        PRECONDITION(CheckPointer(this));
+        SS_POSTCONDITION(CheckPointer(buffer));
+        THROWS;
+    }
+    SS_CONTRACT_END;
+    NewArrayHolder<UTF8> buffer = NULL;
+
+    buffer = new UTF8[GetSize()];
+    strncpy(buffer, GetUTF8(), GetSize());
+
+    SS_RETURN buffer.Extract();
+}
+
 //----------------------------------------------------------------------------
 // Return a writeable buffer that can store 'countChars'+1 ansi characters.
 // Call CloseBuffer when done.
@@ -1895,6 +1917,7 @@ FORCEINLINE SString::CIterator SString::End() const
     }
     SS_CONTRACT_END;
 
+    ConvertToIteratable();
     ConvertToIteratable();
 
     SS_RETURN CIterator(this, GetCount());

diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h
index fdda930a637a33e7ff625238afd8dbc521a0f63c..b46e9c7a2f7949b625bbc339cfadf81f6dfb1c72 100644 (file)
--- a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h
+++ b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h
@@ -1137,12 +1137,34 @@ ep_rt_entrypoint_assembly_name_get_utf8 (void)
 
        // get the name from the host if we can't get assembly info, e.g., if the runtime is
        // suspended before an assembly is loaded.
-       SString assembly_name;
-       if (HostInformation::GetProperty (HOST_PROPERTY_ENTRY_ASSEMBLY_NAME, assembly_name))
-               return assembly_name.GetUTF8 ();
+       // We'll cache the value in a static function global as the caller expects the lifetime of this value
+       // to outlast the calling function.
+       static const ep_char8_t* entrypoint_assembly_name = nullptr;
+       if (entrypoint_assembly_name == nullptr)
+       {
+               const ep_char8_t* entrypoint_assembly_name_local;
+               SString assembly_name;
+               if (HostInformation::GetProperty (HOST_PROPERTY_ENTRY_ASSEMBLY_NAME, assembly_name))
+               {
+                       entrypoint_assembly_name_local = reinterpret_cast<const ep_char8_t*>(assembly_name.GetCopyOfUTF8String ());
+               }
+               else
+               {
+                       // fallback to the empty string
+                       // Allocate a new empty string here so we consistently allocate with the same allocator no matter our code-path.
+                       entrypoint_assembly_name_local = new ep_char8_t [1] { '\0' };
+               }
+               // Try setting this entrypoint name as the cached value.
+               // If someone else beat us to it, free the memory we allocated.
+               // We want to only leak the one global copy of the entrypoint name,
+               // not multiple copies.
+               if (InterlockedCompareExchangeT(&entrypoint_assembly_name, entrypoint_assembly_name_local, nullptr) != nullptr)
+               {
+                       delete[] entrypoint_assembly_name_local;
+               }
+       }
 
-       // fallback to the empty string
-       return reinterpret_cast<const ep_char8_t*>("");
+       return entrypoint_assembly_name;
 }
 
 static