A potential out-of-bound read in RuntimeInvokeHostAssemblyResolver

[JJLovesLife]

Description

In AssemblyLoadContext, there is a check to avoid the loaded assembly to be reflection emitted assembly. However, the native code checking this seems to be contains a potential out-of-bound memory read, caused by the mismatching between RuntimeAssembly and RuntimeAssemblyBuilder.

Here is the code analysis:

runtime/src/coreclr/vm/appdomain.cpp

Lines 4546 to 4548 in 0dac635

	// We were able to get the assembly loaded. Now, get its name since the host could have
	// performed the resolution using an assembly with different name.
	DomainAssembly *pDomainAssembly = _gcRefs.oRefLoadedAssembly->GetDomainAssembly();

This code here _gcRefs.oRefLoadedAssembly could be returned from AssemblyLoadContext.Load which is controlled by user, so _gcRefs.oRefLoadedAssembly of type Assembly could actually be RuntimeAssemblyBuilder.
But ->GetDomainAssembly() access offset 0x20 of the Assembly object, where RuntimeAssemblyBuilder has only a size of 0x20(don't count header). So access domain assembly on RuntimeAssemblyBuilder cause it reads the end of the objects, which normally is the header of _internalAssembly of RuntimeAssemblyBuilder, but potentially be any object's header or even something else if a GC happens between this lines code.
If that happens, following access of DomainAssembly causes a memory access violation.

runtime/src/coreclr/vm/appdomain.cpp

Lines 4551 to 4558 in 0dac635

	if (!pDomainAssembly)
	{
	// Reflection emitted assemblies will not have a domain assembly.
	fFailLoad = true;
	}
	else
	{
	pLoadedPEAssembly = pDomainAssembly->GetPEAssembly();

Reproduction Steps

using System.Reflection;
using System.Runtime.Loader;
using System.Reflection.Emit;

new MyLoadContext().LoadFromAssemblyName(new("DynamicAssemblyExample"));

class MyLoadContext : AssemblyLoadContext
{
	protected override Assembly? Load(AssemblyName assemblyName)
	{
		var aName = new AssemblyName("DynamicAssemblyExample");
		AssemblyBuilder ab =
			AssemblyBuilder.DefineDynamicAssembly(
				aName,
				AssemblyBuilderAccess.Run);
		// Here I am forcing memory just after RuntimeAssemblyBuilder to be non-zero by cause _internalAssembly's obj header to be != 0.
		// But that memory could potentially be non-zero if this is a GC happens during `DefineDynamicAssembly`
		typeof(AssemblyBuilder).Assembly.GetType("System.Reflection.Emit.RuntimeAssemblyBuilder", true)!.GetField("_internalAssembly", BindingFlags.NonPublic | BindingFlags.Instance)!.GetValue(ab)!.GetHashCode();
		return ab;
	}
}

Result:

Fatal error. Internal CLR error. (0x80131506)
   at System.Reflection.RuntimeAssembly.InternalLoad(System.Reflection.AssemblyName, System.Threading.StackCrawlMark ByRef, System.Runtime.Loader.AssemblyLoadContext, System.Reflection.RuntimeAssembly, Boolean)
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyName(System.Reflection.AssemblyName)
   at Program.<Main>$(System.String[])

Expected behavior

The check within RuntimeInvokeHostAssemblyResolver should aware that _gcRefs.oRefLoadedAssembly could be any type inherited from Assembly

Actual behavior

Code in RuntimeInvokeHostAssemblyResolver assume _gcRefs.oRefLoadedAssembly to be RuntimeAssembly or has a simlar memory layout.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

[jkotas]

Regression introduced by https://github.com/dotnet/runtime/pull/53219/files#diff-f5eca13052d21c2368845320c3ab31cfd35851340bfb351e9b21b6fae755e363R660 .

[JJLovesLife]

Regression introduced by https://github.com/dotnet/runtime/pull/53219/files#diff-f5eca13052d21c2368845320c3ab31cfd35851340bfb351e9b21b6fae755e363R660 .

I saw this was to improve the name validation for reflection dynamic assemblies. FYI, I also encountered same kind of confusion exception saying name isn't match when I try to reproduce with PersistedAssemblyBuilder which introduced in .NET 9. Just a reminder to cover that and don't introduce similar issues.