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

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

Author: 3 people

src/coreclr/inc/sstring.h

@@ -518,6 +518,9 @@ class EMPTY_BASES_DECL SString : private SBuffer
     //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();
 

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());

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